starknet-specs icon indicating copy to clipboard operation
starknet-specs copied to clipboard

Apply prettier, upgrade Node, add PR template

Open FabijanC opened this issue 1 year ago • 2 comments

  • Supersedes #241
  • Introduced prettier as devDependency in package.json
    • Replaced .editorconfig with .prettierrc
  • Applied prettier to all .js, .json, .md files.
  • Added formatting commands:
    • npm run format - to be invoked locally
    • npm run format_check - to be invoked as a CI step - updated validate.yml
  • Upgraded node version used in CI from v16 to v20
  • Added .github/pull_request_template.md to serve as a reminder to contributors

FabijanC avatar Dec 03 '24 15:12 FabijanC

Thanks for the much-needed PR

We should merge it as the last PR of 0.8.0 so we don't create too many conflicts with open PRs.

Question: is this with the formatting already applied to all the files? or only partially? (asking because I'm getting file changes after running the scripts)

Also, What should be the output of format-check? seems to just spit out all of the current files.. Or maybe something is misconfigured for me locally? I'm not quite sure how prettier behaves with local/global configs, and with local/global .editorconfigs (which is one of the reasons I didn't opt into it right from the start)

amanusk avatar Dec 19 '24 08:12 amanusk

Thanks for the review @amanusk!

I'm ok with this being the last PR of 0.8.0

The formatting is already applied, I didn't get any new changes besides the ones resulting from merging with the target branch where my other PR was recently merged and needed formatting. Those changes can be seen in one of my last commits: https://github.com/starkware-libs/starknet-specs/pull/258/commits/4ff04b581b7e3be4cdb5a65201840aee8e8f688c. Perhaps the horrible output on npm run format_check led to confusion?

I improved the output of formatting check to make it user friendly and renamed the command from format-check to format_check in this commit: https://github.com/starkware-libs/starknet-specs/pull/258/commits/992b6384fee1d1801f11fe8a6c6c28f62bfc8991.

Prettier relies on the locally present .editorconfig. The changes made by your IDE on file save may vary, depending on how the IDE formatting is configured. But even if there are such differences, if the last thing before staging your work and committing is applying the formatting with npm run format, then there should be no fear. To ensure this, I recommend creating an executable file at .git/hooks/pre-commit and adding the following content to it (based on validate.yml):

#!/bin/sh

set -euo pipefail

npm run validate_all
npm run check_component_uniqueness
npm run format_check

That way, all CI checks shall be performed before committing, disallowing the commit if anything fails.

FabijanC avatar Dec 19 '24 08:12 FabijanC