Web Site Minor Clean Up #386
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Removed Tournaments.pug Page and all corresponding pages due to its main use which displayed the FAF Challonge did not work and the Challonge API was causing long load times. - Update Competitive tab to link to the Global ranks & added in 4v4 Full share - Minor Page Clean ups - Updated Main page and added GOG buy Link - Updated Account Register due to still having Steam link mandatory.
Rackover
reviewed
May 29, 2022
| locals.apiURL = process.env.API_URL; | ||
|
|
||
| fs.readFile('members/4v4.json', 'utf8', function (err, data) { | ||
| if(err) return; |
There was a problem hiding this comment.
Log error when it happens, do not fail silently 🙂
| fs.readFile('members/4v4.json', 'utf8', function (err, data) { | ||
| if(err) return; | ||
| locals.members = JSON.parse(data); | ||
| locals.lastPage = Math.ceil(locals.members.length / 100); |
There was a problem hiding this comment.
local.members not checked, what if JSON.parse fails?
| fs.readFile('members/4v4.json', 'utf8', function (err, data) { | ||
| if(err) return; | ||
| locals.members = JSON.parse(data); | ||
| locals.lastPage = Math.ceil(locals.members.length / 100); |
There was a problem hiding this comment.
100 needs to be a variable / constant defined somewhere, no magic/absolutes (it's the page size)
| return; | ||
| } | ||
|
|
||
| const ratings = models.sync(JSON.parse(body)); |
There was a problem hiding this comment.
Check JSON.parse or wrap it in a try catch
|
|
||
| request(process.env.API_URL + "/data/leaderboardRating?include=player&sort=-rating&filter=leaderboard.id==4;updateTime=ge=" + | ||
| pastMonth.format("YYYY-MM-DDTHH:mm:ss") + "Z", function (error, response, body) { | ||
| if (error || response.statusCode > 210) { |
There was a problem hiding this comment.
is that the traditional NPM request we're using here? It's heavily deprecated if so, we should probably not use that anymore (2020)
probably not in the scope of that PR but good to keep in mind
| let fs = require('fs'); | ||
| let request = require("request"); | ||
| let fs = require("fs"); | ||
| let moment = require("moment"); |
| @@ -2,7 +2,8 @@ require("dotenv").config(); | |||
|
|
|||
| let jsonapi = require("json-api-models"); | |||
| let request = require("request"); | |||
| let fs = require('fs'); | |||
| let fs = require("fs"); | |||
| let moment = require("moment"); | |||
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Removed Tournaments.pug Page and all corresponding pages due to its main use which displayed the FAF Challonge did not work and the Challonge API was causing long load times.
Update Competitive tab to link to the Global ranks & added in 4v4 Full share
Minor Page Clean ups
Updated Main page and added GOG buy Link
Updated Account Register due to still having Steam link mandatory.