Skip to content

Conversation

mathisve
Copy link
Contributor

Added some questionable (but working) test code.
You can run it by doing go test.

Also added the /V1/fetch route to get the latest 10 bumped servers.

@mathisve mathisve added documentation Improvements or additions to documentation enhancement New feature or request hacktoberfest labels Oct 11, 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 good work. Only a couple of minor things that need to be reviewed.

Both the "TEST_INTERVAL" and "PORT" constants are not being used throughout the source code.

Outside of this PR, we should consider replacing the occurrences of "localhost" with a constant placeholder to facilitate distributing this application in a cloud environment.

"guildId": guildId,
})

resp, _ := http.Post("http://localhost:8080/V1/bump", "application/json", bytes.NewBuffer(reqBody))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See previous comment.

api/src/main.go Outdated
const (
PORT = ":8080"
BUMP_INTERVAL = 60 // 1 minute in seconds
TEST_INTERVAL = 2
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This constant has not been implemented/used.

@mathisve
Copy link
Contributor Author

I almost forgot to mention but I added documentation for the fetch route.

@mathisve mathisve requested a review from Travis-Owens October 12, 2020 16:00
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, good job on resolving the issues. Looking forward, we should add more commenting to the source code, as this will increase readability and maintainability.

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.

Looks good 👍

@mathisve mathisve merged commit b737d0f into development Oct 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation enhancement New feature or request hacktoberfest hacktoberfest-accepted

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants