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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

put back info of markdown nodes in tocnode #103

Merged
merged 2 commits into from Mar 6, 2019

Conversation

Projects
None yet
3 participants
@jckr
Copy link
Contributor

commented Mar 6, 2019

This implements what's described in #101.

Currently, in luma.gl, table-of-contents.json doesn't match the contents of the folders. As a result there are some mismatches, for instance all the loaders docs have moved to loaders.gl.

But besides that, the TOC has all the entries even "the ones that got away" 馃帀

@jckr jckr requested a review from ibgreen Mar 6, 2019

// but we can inject it afterwards
const nodeToEdit = parseToc([tocNode], relPath);
if (nodeToEdit) {
nodeToEdit.childMarkdownRemark = {

This comment has been minimized.

Copy link
@jckr

jckr Mar 6, 2019

Author Contributor

the regular toc node generation process adds the full content of each markdown node to the toc. we don't need as much. The app will only use the title and slug of the corresponding markdown node for each toc entry.

This comment has been minimized.

Copy link
@ibgreen

ibgreen Mar 6, 2019

Contributor

Suggestion: When I feel the need to add a comment like that to the PR to guide the reader, I usually realize that I would have been better of just adding it to the code so that future maintainers also benefit...

@ibgreen

ibgreen approved these changes Mar 6, 2019

Copy link
Contributor

left a comment

This is very encouraging. If this works it fixes one of the last remaining major issues with ocular-gatsby, the async toc/node generation issue.

@@ -9,6 +9,7 @@ module.exports.processNewDocsJsonNode = function processNewDocsJsonNode({
}, docNodes) {
traverseTableOfContents(node.chapters, docNodes, 1);
tableOfContents = node;
return node;

This comment has been minimized.

Copy link
@ibgreen

ibgreen Mar 6, 2019

Contributor

Looks like you left code after the return statement... can't imagine the linter being happy with this...

This comment has been minimized.

Copy link
@jckr

jckr Mar 6, 2019

Author Contributor

ok I took a pass on that file to make it more linter friendly

addSourceInstanceName(...arguments);
processed = true;
break;
case `text/markdown`:

This comment has been minimized.

Copy link
@ibgreen

ibgreen Mar 6, 2019

Contributor

I have said this before, but I think we need to get your new editor setup so it reads formatting rules from files in the repo, better avoid unrelated formatting changes. Time to implement prettier.

This comment has been minimized.

Copy link
@jckr

jckr Mar 6, 2019

Author Contributor

i'm all about settling for common prettier rules. we have a .eslintrc.json file in the repo but no .prettierrc or equivalent

This comment has been minimized.

Copy link
@ibgreen

ibgreen Mar 6, 2019

Contributor

In the same repo we have a module ocular-dev-tools, I believe it includes prettier in its scripts, we should update.

};

function parseToc(queue, entry) {
// this function returns a node in the TOC that has an entry corresponding to

This comment has been minimized.

Copy link
@ibgreen

ibgreen Mar 6, 2019

Contributor

Nit: Comment that relates to whole function goes above the function keyword

// but we can inject it afterwards
const nodeToEdit = parseToc([tocNode], relPath);
if (nodeToEdit) {
nodeToEdit.childMarkdownRemark = {

This comment has been minimized.

Copy link
@ibgreen

ibgreen Mar 6, 2019

Contributor

Suggestion: When I feel the need to add a comment like that to the PR to guide the reader, I usually realize that I would have been better of just adding it to the code so that future maintainers also benefit...

@jckr jckr merged commit 5db7fad into master Mar 6, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can鈥檛 perform that action at this time.