-
Notifications
You must be signed in to change notification settings - Fork 4
Added bump endpoint #1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
mathisve
commented
Oct 4, 2020
- Added the /V1/bump endpoint
- Added the temporary JSON Store ("database")
- Added /V1/bump endpoint documentation
blacksmithop
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
API looks good, I'm up for merging.
Travis-Owens
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Mathisco-01, Overall great job on the code in this commit.
There are a few consistent issues throughout the commit:
- Non-descriptive variable naming
- Inconsistent indentation (editor settings?)
- General lack of detailed comments
As this project progresses, I think we'll want to make commits much smaller and more focused on individual task. However, at the beginner there is just a lot of framework related task to contend with that bloat commit scopes.
|
|
||
| const ( | ||
| PORT = ":8080" | ||
| BUMP_INTERVAL = 60 // 1 minute in seconds |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Inconsistent indentation
| log.Print(err) | ||
| } else { | ||
| log.Print("'Database' successfully restored!") | ||
| log.Print(fmt.Sprintf("%v guilds in 'database'", len(gs.Guilds))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Inconsistent indentation
| return | ||
| } | ||
|
|
||
| if !gs.GuildInStore(guild) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if { if { if { }}} Can this be simplified?
| func HandleRequests() { | ||
| router := mux.NewRouter().StrictSlash(true) | ||
| router.HandleFunc("/V1/bump", middleware(BumpGuild)).Methods("POST") | ||
| log.Print(fmt.Sprintf("Now serving: localhost%s", PORT)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Inconsistent indentation
| router := mux.NewRouter().StrictSlash(true) | ||
| router.HandleFunc("/V1/bump", middleware(BumpGuild)).Methods("POST") | ||
| log.Print(fmt.Sprintf("Now serving: localhost%s", PORT)) | ||
| err := http.ListenAndServe(PORT, router) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Inconsistent indentation
|
|
||
| func LoadStore() (guilds []Guild, err error) { | ||
| // Open store.json file | ||
| file, err := os.OpenFile(STORE_FILE_NAME, os.O_CREATE, 0644) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Inconsistent indentation
| } | ||
|
|
||
| // Read file into contents | ||
| contents, err := ioutil.ReadAll(file) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Inconsistent indentation
|
|
||
| gs.Guilds = sortGuilds(gs.Guilds) | ||
|
|
||
| g, err := json.Marshal(gs.Guilds) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"g" = guild?
| return err | ||
| } | ||
|
|
||
| func sortGuilds(guilds []Guild) []Guild { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Comments? What purpose does this function accomplish?
| return guilds | ||
| } | ||
|
|
||
| func (gs *GuildStore) GuildInStore(guild Guild) (bool) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add some comments to explain the difference between GuildStore and storedGuild
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This reminded me of the fact that since I do not work in Go, my approvals will mostly be based on how I could interact it with the api in my local setup. I'd suggest having someone who does work with Go to approve future PR's.