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

docs: test for cli options being in sync between node_options.cc and cli.md #51623

Merged
merged 1 commit into from
Mar 7, 2024
Merged

docs: test for cli options being in sync between node_options.cc and cli.md #51623

merged 1 commit into from
Mar 7, 2024

Conversation

Uzlopak
Copy link
Contributor

@Uzlopak Uzlopak commented Jan 31, 2024 •
edited
Loading

This PR has the goal to ensure that we have all cli options documented.

@nodejs-github-bot nodejs-github-bot added the needs-ci PRs that need a full CI run. label Jan 31, 2024
@Uzlopak Uzlopak force-pushed the test-cli-md-completeness branch 2 times, most recently from 995c3d2 to a0599a5 Compare February 4, 2024 15:35
@Uzlopak Uzlopak marked this pull request as ready for review February 4, 2024 15:35
@Uzlopak
Copy link
Contributor Author

Uzlopak commented Feb 4, 2024

@nodejs/documentation
@nodejs/testing

@Uzlopak
Copy link
Contributor Author

Uzlopak commented Feb 14, 2024

@mcollina

How can I promote this PR?

@Uzlopak
Copy link
Contributor Author

Uzlopak commented Feb 22, 2024

@H4ad

Any idea how i can get more attention to this PR? :)

@H4ad
Copy link
Member

H4ad commented Feb 22, 2024

I think it's a good PR but maybe having some content in each new CLI option will be better than just leaving it empty.

The description of each option doesn't need to be extensive but at least provides some indications about what it does.

You probably can find more information about each flag by looking the PR that adds that flag, for example: #50899

@nodejs/documentation Pinging again just to get more feedback.

@Uzlopak
Copy link
Contributor Author

Uzlopak commented Feb 22, 2024

I am happy to add more content, but I wanted to have some feedback before I start writing text which could get rejected.

mcollina
mcollina approved these changes Feb 22, 2024
View reviewed changes
Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@mcollina mcollina added the request-ci Add this label to start a Jenkins CI on a PR. label Feb 22, 2024
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Feb 22, 2024
@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/57292/

@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/57361/

H4ad
H4ad approved these changes Feb 23, 2024
View reviewed changes
Copy link
Member

@H4ad H4ad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we didn't get feedback from the documentation team, maybe we should iterate on the missing documentation over time, at least now we know that there are a lot of flags that still need some documentation to be created.

Thanks for the PR.

@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/57510/

@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/57521/

@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/57522/

@richardlau richardlau added the request-ci Add this label to start a Jenkins CI on a PR. label Mar 1, 2024
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Mar 1, 2024
@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/57530/

@H4ad
Copy link
Member

H4ad commented Mar 1, 2024

@Uzlopak Can you rebase? I think there is fixes on CI that this current version is not getting.

@Uzlopak
Copy link
Contributor Author

Uzlopak commented Mar 1, 2024 •
edited
Loading

should i wait till the CI ran through or should I do it proactively now?

done

@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/57610/

@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/57611/

@Uzlopak
Copy link
Contributor Author

Uzlopak commented Mar 7, 2024

@H4ad

Are the failures related to my PR?

@Uzlopak
Copy link
Contributor Author

Uzlopak commented Mar 7, 2024

Yay, everything green

@H4ad H4ad added the commit-queue Add this label to land a pull request using GitHub Actions. label Mar 7, 2024
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Mar 7, 2024
@nodejs-github-bot nodejs-github-bot merged commit 999c6b3 into nodejs:main Mar 7, 2024
50 checks passed
@nodejs-github-bot
Copy link
Collaborator

Landed in 999c6b3

rdw-msft pushed a commit to rdw-msft/node that referenced this pull request Mar 26, 2024
@Uzlopak @rdw-msft
doc: test for cli options
67b2178 
PR-URL: nodejs#51623
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Vinícius Lourenço Claro Cardoso <contact@viniciusl.com.br>
@Uzlopak Uzlopak deleted the test-cli-md-completeness branch April 7, 2024 06:16
@RafaelGSS RafaelGSS mentioned this pull request Apr 15, 2024
Merged
@marco-ippolito marco-ippolito added the dont-land-on-v20.x PRs that should not land on the v20.x-staging branch and should not be released in v20.x. label May 2, 2024
jcbhmr pushed a commit to jcbhmr/node that referenced this pull request May 15, 2024
@Uzlopak @jcbhmr
doc: test for cli options
3161773 
PR-URL: nodejs#51623
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Vinícius Lourenço Claro Cardoso <contact@viniciusl.com.br>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mcollina mcollina mcollina approved these changes

@H4ad H4ad H4ad approved these changes

@aymen94 aymen94 aymen94 approved these changes

Assignees
No one assigned
Labels
dont-land-on-v20.x PRs that should not land on the v20.x-staging branch and should not be released in v20.x. needs-ci PRs that need a full CI run.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
7 participants
@Uzlopak @H4ad @nodejs-github-bot @mcollina @aymen94 @richardlau @marco-ippolito