semantic-release / github

:octocat: semantic-release plugin to publish a GitHub release and comment on released Pull Requests/Issues
https://www.npmjs.com/package/@semantic-release/github
MIT License
407 stars 125 forks source link

semantic-release/github's CI config is not properly enforcing code format #890

Closed jedwards1211 closed 2 months ago

jedwards1211 commented 2 months ago

I noticed after committing changes to my PR #886 that master seems to not be properly formatted.

The problem is the test script runs lint:*, which includes lint:prettier:fix, so invalid format can get autofixed in CI instead of erroring out. CI should error out so that no format-only changes wind up in the commit history for master.

How do you want me to fix this? I could either replace lint:* with lint:prettier, lint:lockfile etc (excluding lint:prettier:fix), or I could rename lint:prettier:fix to prettier:fix.

travi commented 2 months ago

unsure about the issues on the default branch, but lint:* does not execute lint:prettier:fix. lint:** would be needed for that, as described in https://github.com/bcomnes/npm-run-all2/blob/master/docs/npm-run-all.md#glob-like-pattern-matching-for-script-names

babblebey commented 2 months ago

The problem is the test script runs lint:*, which includes lint:prettier:fix, so invalid format can get autofixed in CI instead of erroring out. CI should error out so that no format-only changes wind up in the commit history for master.

Hey @jedwards1211, in confirmation to what @travi as said...

...only the following lint related scripts are ran when the lint:** is executed and they're intended as part of the test to check the code is property formatted in CI.

The lint:prettier:fix isn't executed as part of the test script, it is instead reserved to be manually executed to fix a code formatting/lint error when its caught in the test script.

babblebey commented 2 months ago

So, @jedwards1211

If you're experiencing the lint error in your PR #886, the test script isn't meant to help address it.

You can address it by running the lint fix command and committing the changes.

npm run lint:prettier:fix

Also, do not forget to run npm run test afterward to confirm there are no other lint errors or failing tests.

jedwards1211 commented 2 months ago

Okay didn't realize that about lint:*. I was confused why there were unrelated format changes in my PR, but it seems like my format on save in VSCode is somehow using different format settings than npm run lint:prettier:fix, which is odd because it should be configured to use the verison of prettier installed in the project...

jedwards1211 commented 2 months ago

Okay looks like the VSCode prettier plugin has this bad behavior:

["INFO" - 11:13:11 AM] No local configuration (i.e. .prettierrc or .editorconfig) detected, falling back to VS Code configuration
["INFO" - 11:13:11 AM] Prettier Options:
{
  "arrowParens": "always",
  "bracketSpacing": true,
  "endOfLine": "lf",
  "htmlWhitespaceSensitivity": "css",
  "insertPragma": false,
  "singleAttributePerLine": false,
  "bracketSameLine": false,
  "jsxBracketSameLine": false,
  "jsxSingleQuote": false,
  "printWidth": 80,
  "proseWrap": "preserve",
  "quoteProps": "as-needed",
  "requirePragma": false,
  "semi": true,
  "singleQuote": false,
  "tabWidth": 2,
  "trailingComma": "es5",
  "useTabs": false,
  "embeddedLanguageFormatting": "auto",
  "vueIndentScriptAndStyle": false,
  "filepath": "/Users/andy/gh/github/lib/definitions/errors.js",
  "parser": "babel"
}

No one should have ever designed it to work this way when prettier is installed in the project, it should just run prettier in the project and let it use its own options.