Shorter nodeserver resource names#8
Merged
SarahZum merged 3 commits intoSarahZum:masterfrom Aug 11, 2017
Merged
Conversation
I missed this on first glance, too - there's a comma instead of a dot!
…ls and articles The default URL has primo/browzine/ already in it, so if we use that, we already identify this URL as being related to browzine. Also, this entire repo has browzine in the name, so the node server being for browzine articles and journals should be fairly clear - and the README should make that clear.
…zineLibraryID It'll be a little clearer, especially for systems that may have many env vars on them
Owner
|
Thanks Karl, makes sense to me! |
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Hi @SarahZum !
After setting things up on my own, thought a few tweaks to names of things might help this be a little more straightforward:
browzineis in the/primo/browzine/browzineJournalsURL twice, so how about we eliminate the second reference to it and instead have/primo/browzine/browzineJournals?browzineArticleInContext, except I simplified it down even further toarticlessince the resource being requested really is just an article - it just so happens to include an "article in context" URL, but it also has just general information about the article, toobrowzinein the name so systems with other env variables can more clearly separate these two from other varsI think all of these things simplify the code a little bit, and things like the env vars hopefully help the long-term maintenance for anyone who installs this on their own server, too.
What do you think?
Thanks!