Simplify pages' configuration format #9
Closed
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
this seems significantly less noisy and generally easier to scan the implementation could likely be improved WRT elegance and efficiency
FND
force-pushed
the
simple-config
branch
2 times, most recently
from
fa4ba24
to
4cd957a
Compare
while syntactically this might look more complicated, it's actually more explicit and thus easier to understand (less context required)
moonglum
reviewed
Nov 9, 2018
| @@ -3,7 +3,7 @@ let path = require("path"); | |||
|
|
|||
| module.exports = { | |||
| styleguide: [{ | |||
| source: "./pages.json", | |||
| source: "./pages.js", | |||
There was a problem hiding this comment.
Different idea:
source: require("./pages")This way, we can reserve the String for the Roman-style configuration where you just provide a path and we will detect all md Files for you and deduce the page structure from there. So the contract is:
- Either provide a path, then we will do it Roman-style
- Or provide a data structure, then we will do it FND-style
With the second option, you can either a) put it in the faucet.config.js directly b) put it in a JS file and require it or c) put it in a JSON file and require it
There was a problem hiding this comment.
I like it!
Hover, I'm currently deep in the weeds preparing the next PR, which directly affects this processing:
https://github.com/FND/styleguide/blob/decf0a6a22bf5cbaa8eee033121893485ee83121/lib/site.js#L56
Would you mind putting off this change until that part's stable? It seems easier to adjust (more neatly encapsulated) over there and would save me the trouble of rebasing with conflicts.
|
Reboot of this in #13 |
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.
please review commits individually
a few semi-related observations:
faucet.config.js) - perhaps something about resources or routes?faucet.config.js, which strikes me as odd/inconsistent