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.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

tools: added remark-frontmatter #38717

Closed
wants to merge 1 commit into from
Closed

tools: added remark-frontmatter #38717

wants to merge 1 commit into from

Conversation

benhalverson
Copy link
Member

remark-frontmatter allows the use of frontmatter metadata in markdown
files. The purpose of this change is so nodejs.dev can parse the docs with graphql and frontmatter markdown.

In my brief testing adding this change doesn't break nodejs.org

@github-actions github-actions bot added doc Issues and PRs related to the documentations. tools Issues and PRs related to the tools directory. labels May 18, 2021
@targos
Copy link
Member

targos commented May 18, 2021 •
edited
Loading

Does it change anything in the built docs or it's just to prepare for future changes?

@benhalverson
Copy link
Member Author

This is for future changes. It's not changing any existing docs at the moment.

@targos
Copy link
Member

targos commented May 18, 2021

@nodejs/documentation

@aduh95
Copy link
Contributor

aduh95 commented May 18, 2021

What would be the use of this in our docs? Do you have an example would be nice, or maybe a link to a thread where this was discussed?

@benhalverson
Copy link
Member Author

@aduh95 This change is for nodejs.dev that will eventually replace nodejs.org. The nodejs.org docs site would ignore this change.
https://github.com/nodejs/nodejs.dev
An example

---
title: Buffer
url: buffer
category: docs
lang: en
---

<current buffer content...>

it's been discussed in website-redesign meetings. I've asked in the openJS slack here.

I'll have a working demo later today.

@benhalverson
Copy link
Member Author

Here's a demo without the frontmatter changes. I'm using the i18n npm package from node/i18n they have a nightly cron job that gets the latest docs.
PR: nodejs/nodejs.dev#1344
Demo: https://staging.nodejs.dev/1344/docs-demo

with this front matter change I can more easily add a slug/id after /docs. An existing implementation we have is for the blog posts that don't yet exist here.

https://github.com/nodejs/nodejs.dev/blob/master/gatsby-node.js#L136-L141

@aduh95
Copy link
Contributor

aduh95 commented May 24, 2021

@benhalverson are you planning to open other PRs to add frontmatter metadata headers to the doc/api markdown files in the near future? I'd prefer we hold this PR until there's at least one PR ready to change an actual markdown file.

@benhalverson
Copy link
Member Author

@aduh95 Yes I plan to do the doc PR's. Do I need to create a issue first?

@aduh95
Copy link
Contributor

aduh95 commented May 24, 2021

I don't think an issue is necessary – or at least, it wouldn't be very useful in this repo imho given this kind of change is visible to nodejs.dev users only.

@zeke
Copy link
Contributor

zeke commented Jun 25, 2021

cc @nodejs/i18n 👀

@RichardLitt
Copy link
Contributor

Sounds like a good move to me, as far as i18n goes.

@benhalverson
Copy link
Member Author

Thanks for the approval I'll get that merge conflict fixed!

@benhalverson
tools: added remark-frontmatter
386fb93 
remark-frontmatter allows the use of frontmatter metadata in markdown
files
@benhalverson
Copy link
Member Author

Do I need to do anything about the failed checks?

@DerekNonGeneric
Copy link
Contributor

@benhalverson, that CI failure does not appear to be related, and am not clear on why that happened, but the PR looks good to me, so let's get this thing landed for you already.

@DerekNonGeneric DerekNonGeneric added the commit-queue Add this label to land a pull request using GitHub Actions. label Jul 14, 2021
@github-actions github-actions bot added commit-queue-failed An error occurred while landing this pull request using GitHub Actions. and removed commit-queue Add this label to land a pull request using GitHub Actions. labels Jul 14, 2021
@github-actions
Copy link
Contributor

Commit Queue failed
- Loading data for nodejs/node/pull/38717
✔  Done loading data for nodejs/node/pull/38717
----------------------------------- PR info ------------------------------------
Title      tools: added remark-frontmatter (#38717)
   ⚠  Could not retrieve the email or name of the PR author's from user's GitHub profile!
Branch     benhalverson:docs-frontmatter -> nodejs:master
Labels     commit-queue, doc, tools
Commits    1
 - tools: added remark-frontmatter
Committers 1
 - Ben Halverson 
PR-URL: https://github.com/nodejs/node/pull/38717
Reviewed-By: James M Snell 
Reviewed-By: Daijiro Wachi 
Reviewed-By: Derek Lewis 
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/38717
Reviewed-By: James M Snell 
Reviewed-By: Daijiro Wachi 
Reviewed-By: Derek Lewis 
--------------------------------------------------------------------------------
   ⚠  Commits were pushed since the last review:
   ⚠  - tools: added remark-frontmatter
   ℹ  This PR was created on Tue, 18 May 2021 06:07:57 GMT
   ✔  Approvals: 3
   ✔  - James M Snell (@jasnell) (TSC): https://github.com/nodejs/node/pull/38717#pullrequestreview-701554823
   ✔  - Daijiro Wachi (@watilde): https://github.com/nodejs/node/pull/38717#pullrequestreview-701558858
   ✔  - Derek Lewis (@DerekNonGeneric): https://github.com/nodejs/node/pull/38717#pullrequestreview-701575341
   ✖  Last GitHub CI failed
   ℹ  Green GitHub Actions CI is sufficient
--------------------------------------------------------------------------------
   ✔  Aborted `git node land` session in /home/runner/work/node/node/.ncu
https://github.com/nodejs/node/actions/runs/1028835470

@DerekNonGeneric DerekNonGeneric added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Jul 14, 2021
jasnell pushed a commit that referenced this pull request Jul 19, 2021
@benhalverson @jasnell
tools: added remark-frontmatter
74e319f 
remark-frontmatter allows the use of frontmatter metadata in markdown
files

PR-URL: #38717
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
Reviewed-By: Derek Lewis <DerekNonGeneric@inf.is>
@jasnell
Copy link
Member

jasnell commented Jul 19, 2021

Landed in 74e319f

@jasnell jasnell closed this Jul 19, 2021
targos pushed a commit that referenced this pull request Jul 20, 2021
@benhalverson @targos
tools: added remark-frontmatter
5c7ae46 
remark-frontmatter allows the use of frontmatter metadata in markdown
files

PR-URL: #38717
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
Reviewed-By: Derek Lewis <DerekNonGeneric@inf.is>
@BethGriggs BethGriggs mentioned this pull request Jul 26, 2021
Merged
BethGriggs pushed a commit that referenced this pull request Jul 29, 2021
@benhalverson @BethGriggs
tools: added remark-frontmatter
4c32aa0 
remark-frontmatter allows the use of frontmatter metadata in markdown
files

PR-URL: #38717
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
Reviewed-By: Derek Lewis <DerekNonGeneric@inf.is>
targos pushed a commit that referenced this pull request Sep 4, 2021
@benhalverson @targos
tools: added remark-frontmatter
d2c0da2 
remark-frontmatter allows the use of frontmatter metadata in markdown
files

PR-URL: #38717
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
Reviewed-By: Derek Lewis <DerekNonGeneric@inf.is>
@targos targos mentioned this pull request Sep 4, 2021
Merged
@targos targos removed the commit-queue-failed An error occurred while landing this pull request using GitHub Actions. label Sep 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jasnell jasnell jasnell approved these changes

@watilde watilde watilde approved these changes

@waelsy123 waelsy123 waelsy123 approved these changes

@aymen94 aymen94 aymen94 approved these changes

@amorist amorist amorist approved these changes

@DerekNonGeneric DerekNonGeneric DerekNonGeneric approved these changes

Assignees
No one assigned
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. doc Issues and PRs related to the documentations. tools Issues and PRs related to the tools directory.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.