aiur #20
aiur #20
Conversation
moonglum
commented
Nov 20, 2018
•
moonglum
commented
- Fix watch mode
- Move the examples to an example folder
- Rename internally to aiur
- Add LICENSE, COC, CHANGELOG
- Rename repository to aiur
- Setup Travis
- Provide a CLI (CLI executable #14)
- Set language, title and description in aiur.config.js
- Add basic documentation
- Release on NPM as version 0.2.0
| }, | ||
| "dependencies": { | ||
| "aiur": "..", | ||
| "prismjs": "^1.15.0" |
There was a problem hiding this comment.
Users of the package won't need to do that... but for this example it is necessary due to the .. package in the line above.
There was a problem hiding this comment.
You mean because installing a local package like that doesn't pull in its dependencies? What about all the other dependencies then?
Might be easier (as in more straightforward for end users) to reference aiur-the-npm-package instead of trying to optimize via a local path. For my part, I tend to copy such examples into a separate directory anyway (as in npm install foo; cp -R node_modules/foo/sample ./), which wouldn't work here.
There was a problem hiding this comment.
We only need this one dependency here, because we import the CSS from that package and assume that it is located in the node_modules folder.
The optimization to the local path means that I can try out changes via that example folder. When I want to try out if file watching works as expected, it is very easy to just cd in that folder and try it out.
There was a problem hiding this comment.
moonglum
force-pushed
the
aiur
branch
3 times, most recently
from
fc599a0 to
f51dc87
Compare
|
@FND I will merge this now and release version 0.2.0 so I can show it to some people tomorrow. Your review will still be appreciated, and I will adjust it in a follow up. |
| }; | ||
|
|
||
| return { referenceDir: process.cwd(), config, options }; | ||
| }; |
There was a problem hiding this comment.
Is there any particular reason you're not reusing faucet-core's parseCLI; is there something we could (and should) make more flexible over there to promote reuse?
There was a problem hiding this comment.
I'm confused. How should that work? We are adding option to the CLI, how can we use parseCLI then? 🤔
We should definitely mark the boolean flags as boolean. Otherwise you can't accept parameters at the end of the CLI. For example: aiur --watch foo would set "watch" to "foo" without it.
There was a problem hiding this comment.
Well, we could make parseCLI accept an object that's been generated by minimist's parseArgs, for example. (There might be a better abstraction.) But perhaps that's pursuing DRY to an excessive extent; I can't really tell right now - so if you're perfectly comfortable maintaining this, there's no problem to solve.
I'm assuming the boolean thing is essentially a bug report for faucet-core?
There was a problem hiding this comment.
I'll say: Let's keep it like this for now. I'm pretty sure that this is an optimization that doesn't help that much.
| @@ -10,3 +10,6 @@ module.exports = { | |||
| } | |||
| } | |||
| }; | |||
|
|
|||
| exports.title = "Example Styleguide"; | |||
| exports.language = "en"; | |||
There was a problem hiding this comment.
It's easy to miss this down here; I'd move these up top, i.e. above pages.
There was a problem hiding this comment.
You mean all three of them, right? I agree.
|
|
||
| assetURI(filepath) { | ||
| return path.join(this.baseURI, this.assetPath, filepath); // TODO: validate? | ||
| } |
There was a problem hiding this comment.
Where are URIs being generated now? I've noticed Navigation concatenates baseURI and slug, so the lack of encapsulation makes me uncomfortable once more (based on actual real-world experience).
Having said that: It's probably not important right now; we'll deal with it if and when it comes up during actual use.
There was a problem hiding this comment.
Yep, I need to think about that again. I think the current solution is a little fiddly.
| }, | ||
| "dependencies": { | ||
| "aiur": "..", | ||
| "prismjs": "^1.15.0" |
There was a problem hiding this comment.
You mean because installing a local package like that doesn't pull in its dependencies? What about all the other dependencies then?
Might be easier (as in more straightforward for end users) to reference aiur-the-npm-package instead of trying to optimize via a local path. For my part, I tend to copy such examples into a separate directory anyway (as in npm install foo; cp -R node_modules/foo/sample ./), which wouldn't work here.
|
I also found a bug: The |
|
I'm not at all keen on moving the title into Markdown; I consider it essential (i.e. mandatory) metadata. Granted, I've tripped over this page-vs.-site-title issue quite a few times over the years, but it's worth addressing. |
|
I've collected everything I found here to #23. Please redirect all further discussions about the issues raised here over there 😄 |
