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

src: fix node_version.h #50375

Merged
merged 1 commit into from
Oct 24, 2023
Merged

Conversation

joyeecheung
Copy link
Member

@joyeecheung joyeecheung commented Oct 24, 2023

  • Fix the version for main branch as 22.0.0
  • Fix the NODE_VERSION_IS_RELEASE flag for the main branch

This would allow test/parallel/test-release-changelog.js
to pass again on the main branch.

Fixes: #50373

@nodejs-github-bot nodejs-github-bot added the needs-ci PRs that need a full CI run. label Oct 24, 2023
@joyeecheung joyeecheung added the fast-track PRs that do not need to wait for 48 hours to land. label Oct 24, 2023
@github-actions
Copy link
Contributor

Fast-track has been requested by @joyeecheung. Please 👍 to approve.

Copy link
Member

@targos targos left a comment

Choose a reason for hiding this comment

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

Shouldn't the version actually be 22.0.0?

@richardlau
Copy link
Member

Shouldn't the version actually be 22.0.0?

Yes, I think we're missing a step for semver major releases in the release process to bump the major version on main.

@targos
Copy link
Member

targos commented Oct 24, 2023

Yeah, there are two bugs introduced in the file during the release of v21.0.0:

  • major should be set to 22
  • is_release should be set to 0

image

@joyeecheung
Copy link
Member Author

Something is wrong with the test, if I change it to 22.0.0:

❯ out/Release/node test/parallel/test-release-changelog.js
node:fs:453
    return binding.readFileUtf8(path, stringToFlags(options.flag));
                   ^

Error: ENOENT: no such file or directory, open '/Users/joyee/projects/node/doc/changelogs/CHANGELOG_V22.md'
    at Object.readFileSync (node:fs:453:20)
    at Object.<anonymous> (/Users/joyee/projects/node/test/parallel/test-release-changelog.js:42:24)
    at Module._compile (node:internal/modules/cjs/loader:1376:14)
    at Module._extensions..js (node:internal/modules/cjs/loader:1435:10)
    at Module.load (node:internal/modules/cjs/loader:1207:32)
    at Module._load (node:internal/modules/cjs/loader:1023:12)
    at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:135:12)
    at node:internal/main/run_main_module:28:49 {
  errno: -2,
  code: 'ENOENT',
  syscall: 'open',
  path: '/Users/joyee/projects/node/doc/changelogs/CHANGELOG_V22.md'
}

@targos
Copy link
Member

targos commented Oct 24, 2023

@joyeecheung you probably need to fix the IS_RELEASE macro at the same time. I think the bug slipped through because both mistakes happened.

@joyeecheung
Copy link
Member Author

Maybe we should just skip the test when it's not run on a release?

@joyeecheung
Copy link
Member Author

Oh, I see, it was supposed to skip, but NODE_VERSION_IS_RELEASE wasn't updated either.

- Fix the version for main branch as 22.0.0
- Fix the NODE_VERSION_IS_RELEASE flag for the main branch

This would allow test/parallel/test-release-changelog.js
to pass again on the main branch.
@joyeecheung joyeecheung removed the fast-track PRs that do not need to wait for 48 hours to land. label Oct 24, 2023
@joyeecheung joyeecheung added the fast-track PRs that do not need to wait for 48 hours to land. label Oct 24, 2023
@github-actions
Copy link
Contributor

Fast-track has been requested by @joyeecheung. Please 👍 to approve.

@joyeecheung joyeecheung added the request-ci Add this label to start a Jenkins CI on a PR. label Oct 24, 2023
@richardlau richardlau changed the title src: fix NODE_MINOR_VERSION for v21.1.0 src: fix node_version.h Oct 24, 2023
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Oct 24, 2023
@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@aduh95 aduh95 added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Oct 24, 2023
@aduh95 aduh95 added the commit-queue Add this label to land a pull request using GitHub Actions. label Oct 24, 2023
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Oct 24, 2023
@nodejs-github-bot nodejs-github-bot merged commit 89a26b4 into nodejs:main Oct 24, 2023
59 checks passed
@nodejs-github-bot
Copy link
Collaborator

Landed in 89a26b4

@RafaelGSS
Copy link
Member

Sorry about that. As I've mentioned in the release slack channel, something might be missing in our major release guide. I've followed these strictly and something went wrong. I'll triple-check next time :)

image

alexfernandez pushed a commit to alexfernandez/node that referenced this pull request Nov 1, 2023
- Fix the version for main branch as 22.0.0
- Fix the NODE_VERSION_IS_RELEASE flag for the main branch

This would allow test/parallel/test-release-changelog.js
to pass again on the main branch.

PR-URL: nodejs#50375
Fixes: nodejs#50373
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
@targos targos added dont-land-on-v18.x PRs that should not land on the v18.x-staging branch and should not be released in v18.x. dont-land-on-v20.x PRs that should not land on the v20.x-staging branch and should not be released in v20.x. dont-land-on-v21.x labels Nov 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. dont-land-on-v18.x PRs that should not land on the v18.x-staging branch and should not be released in v18.x. dont-land-on-v20.x PRs that should not land on the v20.x-staging branch and should not be released in v20.x. fast-track PRs that do not need to wait for 48 hours to land. needs-ci PRs that need a full CI run.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

test/parallel/test-release-changelog.js is failing on the main branch
7 participants