Skip to content
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.

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

Remove watch chains script #68

Merged
merged 4 commits into from
Aug 26, 2023
Merged

Conversation

andreogle
Copy link
Member

@andreogle andreogle commented Aug 25, 2023

Changes

  1. Removes the yarn dev script which provided the option of watching the chains/ directory and immediately regenerated the chains.ts file. This script is unnecessary and can be dropped in favour of a single development flow for now. Discussion: Fix generate-chains script to work with Prettier v3 #67
  2. Dropped chokidar as a devDependency (used for watching files)
  3. Updates the yarn lint script to include running prettier --check and tsc --noEmit. I re-ran Prettier as part of this PR and noticed that some of the new chain JSON files weren't formatted. The developer will now be forced to run yarn prettier before pushing new changes.
  4. Prettier now also includes .ts files. I completely missed this 🤦🏻‍♂️

@andreogle andreogle requested a review from dcroote August 25, 2023 12:06
@andreogle andreogle self-assigned this Aug 25, 2023
}
},
"browserUrl": "https://basescan.org/"
"name": "Base",
Copy link
Member Author

@andreogle andreogle Aug 25, 2023

Choose a reason for hiding this comment

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

Prettier made these consistent with the other chain files - 2 spaces instead of 4 (the only acceptable number of spaces 😛)

package.json Outdated
"lint": "yarn lint:eslint && yarn lint:prettier && yarn lint:tsc",
"lint:eslint": "eslint . --ext .js,.ts",
"lint:prettier": "prettier --check \"./**/*.{js,md,json}\"",
"lint:tsc": "tsc --noEmit",
Copy link
Member Author

Choose a reason for hiding this comment

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

Not sure if this is necessary or not, since CI now also runs yarn build 🤷🏻‍♂️

Copy link
Contributor

@dcroote dcroote left a comment

Choose a reason for hiding this comment

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

I believe lint:tsc isn't needed because of yarn build (and we don't have tsc --noEmit in other repos), otherwise looks good 👍

@andreogle andreogle merged commit 1860a53 into main Aug 26, 2023
3 checks passed
@andreogle andreogle deleted the chore/remove-watch-chains-option branch August 26, 2023 14:28
Sign up for free to 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.

2 participants