Skip to content
This repository was archived by the owner on May 3, 2022. It is now read-only.

prettier rules #20

Merged
merged 3 commits into from
Apr 10, 2020
Merged

prettier rules #20

merged 3 commits into from
Apr 10, 2020

Conversation

bryanmacfarlane
Copy link
Member

@bryanmacfarlane bryanmacfarlane commented Apr 10, 2020

  • Suggesting a set of prettier rules (same as toolkit repo)
  • Adding prettier to dev packages to ensure we're all using the same version
  • Adding npm run script to ensure devs can run pre-checkin
  • minor audit cmd line fix

After rules merged, contributor will run prettier and enable (uncomment the lines in the workflow)

Copy link

@singingwolfboy singingwolfboy left a comment

Choose a reason for hiding this comment

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

Can we please focus on reviewing and merging #16? I would like to get credit for my work, in the form of a commit on the master branch that has my username attached.

@@ -25,6 +28,7 @@
"@types/jest": "^25.1.4",
"@types/node": "^12.12.31",
"jest": "^25.1.0",
"prettier": "^2.0.4",

Choose a reason for hiding this comment

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

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm fine with major version binding. Semver should hold compat on major versions. It's also consistent with jest and the other repos.

@bryanmacfarlane
Copy link
Member Author

bryanmacfarlane commented Apr 10, 2020

As pointed out in the review of #16, the PR changing all the lines is not viable to review by a core contributor. It forces the contributor to review every line in the project and compare against rules that weren't part of the PR. What should have been part of the PR (the rules, package.json, workflow changes and package scripts) were not part of the PR and what was in the PR is not really usable. I need a core contributor to actually run the format and for the CI to validate it.

However thanks for creating the issue. Creating an issue prior to the PR with a discussion on the rules would have avoided the unnecessary work. Hope that helps.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants