Skip to content

Conversation

@ruyadorno
Copy link
Member

@ruyadornoruyadorno commented Jan 8, 2025

After two consecutive releases in which I mistakenly pushed a broken release commit to main, I'm convinced that the process needs improvement.

This PR proposes to add a reminder to git node release promote to lint markdown files prior to pushing branches to upstream remote.

Refs: nodejs/node@a344300
Refs: nodejs/node#56508

After two consecutive releases in which I mistakely pushed a broken release commit to main, I'm convinced that the process needs improvement. This changeset proposes to add a reminder to `git node release promote` to lint markdown files prior to pushing branches to upstream remote. Refs: nodejs/node@a344300 Refs: nodejs/node#56508
@ruyadornoruyadornoforce-pushed the add-reminder-to-lint-release-commit-on-main branch from ba67a3c to f2ad232CompareJanuary 8, 2025 15:55
@codecov
Copy link

codecovbot commented Jan 8, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 80.08%. Comparing base (4bd6820) to head (f2ad232).

Additional details and impacted files
@@ Coverage Diff @@## main #883 +/- ## ======================================= Coverage 80.08% 80.08% ======================================= Files 39 39 Lines 4676 4676 ======================================= Hits 3745 3745 Misses 931 931 

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Comment on lines +198 to +202
cli.separator();
cli.warn('Reminder: Lint markdown files');
cli.info('You might need to amend the commit if there are issues:');
cli.info('make lint-md');
cli.separator();
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe it makes more sense to move that to the branch where we know there were conflicts (line 168), for the cases where there are no conflicts, it seems safe to skip it

Copy link
MemberAuthor

Choose a reason for hiding this comment

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

That was my original assessment but in the specific occurrence linked in the PR body the bad formatting was introduced by a git automatic resolution, meaning that in theory a successfully cherry-picked commit could potentially still introduce markdown issues.

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't believe it, if I try locally:

git reset a344300bfa146fce9d1f20a71b073263a4720eab^ --hard # rewind just before the faulty commit git cherry-pick v22.13.0^0 # cherry-pick the release commit

All 7 of the YAML sections fixed by nodejs/node@5119049 show up as conflict that must be manually resolved. There might be something else that "does it for you" (like an IDE extension or whatnot), but I'm quite confident git would never

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants

@ruyadorno@anonrig@richardlau@aduh95@RafaelGSS