Skip to content

chore(git-node): Add make lint-md reminder#883

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

Open
ruyadorno wants to merge 1 commit into
base:main
Choose a base branch
from
+6 −0
Open

chore(git-node): Add make lint-md reminder #883

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions lib/promote_release.js
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -195,6 +195,12 @@ export default class ReleasePromotion extends Session {
}
}

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();
Comment on lines +198 to +202

@aduh95aduh95Jan 8, 2025

Copy link
Copy Markdown
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

@ruyadornoruyadornoJan 8, 2025

Copy link
Copy Markdown
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.

@aduh95aduh95Jan 8, 2025

Copy link
Copy Markdown
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


// Push to the remote the release tag, and default, release, and staging branch.
await this.pushToRemote(workingOnNewReleaseCommit);

Expand Down