Closed lukekarrys closed 6 months ago
This may warrant .git-blame-ignore-revs
Will rebase-merge preserve the commit sha?
I checked on npm/cli for a recent multiple commit PR that was rebased and even a rebase merge will also change the sha.
For this PR I will probably land it, temporarily change branch protections, and then force push a new commit to main referencing the correct commit.
For other repos I think the prettier application and git-blame-ignore-revs creation will need to be multiple PRs.
For other repos I think the prettier application and git-blame-ignore-revs creation will need to be multiple PRs.
Let's practice it here then. Don't bother disabling the branch protections. Search for past "lint everything" commits to this repo, including the one from this PR, and make a new PR adding them. That way we can live the process we'll be using going forward.
@wraithgar Good call. Just dropped the .git-blame-ignore-revs
commit from the PR and will make a followup adding the SHA for the prettier formatting commit as well as any other lint commits I can find.
It seems appropriate that @jumoel or @reggi approve this.
This PR contains 4 commits:
prettier
support to@npmcli/template-oss
prettier
on this repo by settingtemplateOSS.prettier: true
770a10c add aDropped from this PR since SHAs won't match after rebase.git-blame-ignore-revs
file referencing the commit from item 3Once this lands, enabling
prettier
on another repo will involve updating to the new version of@npmcli/template-oss
, and then running steps 2-4 which should be handled by a new script in https://github.com/npm/stafftools (or another tool).The most important part of item 1 is the implementation of the
lint
andlintfix
run-scripts. They now look like this:The goal here was to have the same
lint
andlintfix
scripts we currently have also be responsible for runningprettier
. The tradeoff is that if linting fails, then formatting via prettier doesn't run. I think this is ok since it allows for a single script to handle botheslint
andprettier
.We also explored using a combination of
postlint
andpostlintfix
for this, but thenprettier --check
(vialint
->postlint
) would get run beforeprettier --write
which would defeat the purpose ofprettier --write
.Other notable items are:
@github/prettier-config
except for changingbracketSpacing: true
to match how we previously had eslint setup. This also setsarrowParens: "avoid"
which seems like something we've never standardized on previously, so we opted to leave it as configured in the@github
config.eslint-config-prettier
to overwrite all the eslint formatting rules we use. Alternatively we could remove them all from@npmcli/eslint-config
on a semver major of that. I think it's easier to get more things onprettier
before a decision on that is made.