Skip to content

Conversation

@mathisve
Copy link
Contributor

@mathisve mathisve commented Oct 4, 2020

  • Added the /V1/bump endpoint
  • Added the temporary JSON Store ("database")
  • Added /V1/bump endpoint documentation

Copy link
Member

@blacksmithop blacksmithop left a 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.

@mathisve mathisve merged commit d304a0a into development Oct 4, 2020
Copy link
Contributor

@Travis-Owens Travis-Owens left a 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
Copy link
Contributor

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)))
Copy link
Contributor

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) {
Copy link
Contributor

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))
Copy link
Contributor

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)
Copy link
Contributor

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)
Copy link
Contributor

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)
Copy link
Contributor

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)
Copy link
Contributor

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 {
Copy link
Contributor

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) {
Copy link
Contributor

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

Copy link
Member

@blacksmithop blacksmithop Oct 4, 2020

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants