Skip to content
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

Fix/syntax-and-naming #45

Merged
merged 6 commits into from
Jan 24, 2020
Merged

Fix/syntax-and-naming #45

merged 6 commits into from
Jan 24, 2020

Conversation

nmlinaric
Copy link
Member

Removed unnecesarry console logs, improved syntax, changed /diskinfo endpoint to /node/diskinformation and renamed DiskInformation to NodeDiskInformation for more consistent naming.

@morrigan
Copy link
Member

morrigan commented Jan 23, 2020

@nmlinaric Hey, this is fine! However I'd add another change to completely be up to "standards".

Routes should contain visible inheritance in the name. Meaning that since node belong to user, route for fetching a specific user node should be /user/:userId/node/:nodeId. In our case, for now, everything will be prefixed because all routes are user owned. However, in the future we might add some new features that don't require such authorization.

Also, for the future reference, it's more common to user plural (i.e. users) in those cases for routes but we can stick to whatever we want, that's not confusing so much.

@morrigan morrigan mentioned this pull request Jan 24, 2020
@nmlinaric nmlinaric merged commit 50df7b4 into master Jan 24, 2020
@nmlinaric nmlinaric deleted the fix/syntax-and-naming branch January 24, 2020 11:23
@nmlinaric nmlinaric restored the fix/syntax-and-naming branch January 24, 2020 11:23
@nmlinaric nmlinaric deleted the fix/syntax-and-naming branch January 24, 2020 11:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants