Local and external ordering #101
Conversation
Pull Request Test Coverage Report for Build 87
💛 - Coveralls |
src/cmds/build.js
Outdated
| const bundleBar = progress({ clear: true, total: 100 }) | ||
|
|
||
| log('Bundling the Javascript app') | ||
| log('[\u2713] Bundling the Javascript app') |
There was a problem hiding this comment.
Not sure why we would show a checkmark before the operation actually completes. Also log will prefix all messages with "❯" already.
| return warn(`No docs root found for ${s.name || 'source'}, skipping...`) | ||
| } | ||
|
|
||
| // Get external docs config, will be merged with cwd config |
There was a problem hiding this comment.
This functionality should be extended from src/utils/config.js. There is already a _readConfigFile and _getConfigFilename that should be used, since they do a little more.
src/core/index.js
Outdated
| // this gets passed to the theme app | ||
| const props = { | ||
| config, | ||
| mergedConfig, |
There was a problem hiding this comment.
Other files depend on props.config so the key name should stay the same here. Otherwise it will break a lot of stuff.
|
|
||
| return isIndex ? { | ||
| indexLink: hydrated.url, | ||
| order: hydrated.order, |
There was a problem hiding this comment.
Why does this need to be in the actual manifest? It should already be sorted by the time it gets passed to the theme app.
src/core/manifest.js
Outdated
| text: hydrated.title, | ||
| link: hydrated.url, | ||
| order: hydrated.order, | ||
| data: frontMatter |
There was a problem hiding this comment.
Also not sure why you need to pass all the front matter to the nav tree. It already exists in the files part of the manifest, we shouldn't duplicate it. I can give more insight on this decision.
src/core/manifest.js
Outdated
|
|
||
| // Combine and sort local and external nav trees | ||
| const navtree = [...navtreeLocal, ...navtreeExternal] | ||
| .sort((a, b) => { |
There was a problem hiding this comment.
Why does it get sorted twice? Once here and once on line 138.
There was a problem hiding this comment.
The first is only sorting children, not the top level folders—I abstracted it out into a single sorting function.
src/core/static.js
Outdated
| const syspath = require('path') | ||
| const chokidar = require('chokidar') | ||
| const { namespaces } = require('../utils/temp') | ||
| const log = require('../utils/emit') |
There was a problem hiding this comment.
this should be const { log } = require('../utils/emit')
orderconfigorderdata