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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

tools: lint version numbers in doc only in the default branch #37768

Merged
merged 1 commit into from
Mar 16, 2021

Conversation

aduh95
Copy link
Contributor

@aduh95 aduh95 commented Mar 16, 2021

Changelogs are sometimes not backported to release branches; this check
is useful on the master branch, not so much in the other branches.

Refs: #37767

@nodejs-github-bot nodejs-github-bot added the meta Issues and PRs related to the general management of the project. label Mar 16, 2021
@targos
Copy link
Member

targos commented Mar 16, 2021

Changelogs are never backported. I'm okay with this because we usually don't need to validate REPLACEME for other targets than master, but I would still like to understand how we ended up with the issue in #37767 (comment)

@aduh95
Copy link
Contributor Author

aduh95 commented Mar 16, 2021

Hum that clearly doesn't work as expected, it's skipping this PR even though the PR's targeting master.

@aduh95 aduh95 marked this pull request as draft March 16, 2021 16:11
@@ -51,6 +51,7 @@ jobs:
- name: Environment Information
run: npx envinfo
- name: Get release version numbers
if: ${{ github.event.pull_request.base.ref == github.event.pull_request.base.default_branch }}
Copy link
Member

Choose a reason for hiding this comment

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

What does GitHub do here then if the condition fails? Does it skip the entire job, or just this step, in which case what is steps.get-released-versions.outputs.NODE_RELEASED_VERSIONS going to evaluate to in the next step?

Copy link
Member

Choose a reason for hiding this comment

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

it skips only this step

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It skips the step where it parses the changelogs and validate the version number baed on the pattern only – see https://github.com/nodejs/node/runs/2123211599 for an example.

@aduh95 aduh95 marked this pull request as ready for review March 16, 2021 16:20
@aduh95 aduh95 added the fast-track PRs that do not need to wait for 48 hours to land. label Mar 16, 2021
@aduh95
Copy link
Contributor Author

aduh95 commented Mar 16, 2021

fast-track?

The `lint-md` job on GitHub Actions parses the changelogs to determine
if the version numbers referenced in the YAML comments in the docs match
actual releases of Node.js.
Changelogs are sometimes not backported to release branches; this commit
disables changelog parsing on branches other than the default one.

Refs: nodejs#37767

PR-URL: nodejs#37768
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Danielle Adams <adamzdanielle@gmail.com>
Reviewed-By: Ruy Adorno <ruyadorno@github.com>
@aduh95 aduh95 merged commit 901ef1b into nodejs:master Mar 16, 2021
@aduh95
Copy link
Contributor Author

aduh95 commented Mar 16, 2021

Landed in 901ef1b

@aduh95 aduh95 deleted the skip-version-number-test branch March 16, 2021 22:05
danielleadams pushed a commit that referenced this pull request Mar 17, 2021
The `lint-md` job on GitHub Actions parses the changelogs to determine
if the version numbers referenced in the YAML comments in the docs match
actual releases of Node.js.
Changelogs are sometimes not backported to release branches; this commit
disables changelog parsing on branches other than the default one.

Refs: #37767

PR-URL: #37768
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Danielle Adams <adamzdanielle@gmail.com>
Reviewed-By: Ruy Adorno <ruyadorno@github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fast-track PRs that do not need to wait for 48 hours to land. meta Issues and PRs related to the general management of the project.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants