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

chore: enforce prettier and eslint #3014

Merged
merged 7 commits into from
Apr 8, 2024
Merged

chore: enforce prettier and eslint #3014

merged 7 commits into from
Apr 8, 2024

Conversation

mattjennings
Copy link
Contributor

@mattjennings mattjennings commented Apr 7, 2024

===:clipboard: PR Checklist :clipboard:===

  • 📌 issue exists in github for these changes
  • 🔬 existing tests still pass
  • 🙈 code conforms to the style guide
  • 📐 new tests written and passing / old tests updated with new scenario(s)
  • 📄 changelog entry added (or not needed)

==================

See issue #2378

  • Add prettier for code formatting
  • Update eslint config to parse other parts of codebase outside of src/engine, and also remove rules that are handled by prettier
  • Add husky & lint-staged to enforce formatting on git commit
  • Add format step to all and all:ci scripts

I separated the commit that runs the format & lint on all files into #3015 so the config can be easily reviewed here.

Once we're happy with this, we can merge that PR into this one and ignore the commit(s) using git blame --ignore-rev to remove it from git blame history.

@github-actions github-actions bot added the bug This issue describes undesirable, incorrect, or unexpected behavior label Apr 7, 2024
@mattjennings mattjennings mentioned this pull request Apr 7, 2024
5 tasks
@mattjennings mattjennings added chore Maintenance tasks, like upgrading libraries or release tasks and removed bug This issue describes undesirable, incorrect, or unexpected behavior labels Apr 7, 2024
@eonarheim
Copy link
Member

Nice! The CI fails as expected here ;)

Copy link
Member

@eonarheim eonarheim left a comment

Choose a reason for hiding this comment

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

LGTM :shipit:

@mattjennings We expect the CI to balk at the moment because we haven't run this yet correct (at least that's what appears to be happening at the moment)

@mattjennings
Copy link
Contributor Author

@eonarheim yup - that's in #3015. So if you approve that PR we should see the checks run & hopefully pass, then we can merge it into this PR so we can ignore the commit before merging everything altogether into main.

Copy link

cloudflare-workers-and-pages bot commented Apr 8, 2024

Deploying excaliburjs with  Cloudflare Pages  Cloudflare Pages

Latest commit: 8b8ae1f
Status: ✅  Deploy successful!
Preview URL: https://af883000.excaliburjs.pages.dev
Branch Preview URL: https://fix-2378.excaliburjs.pages.dev

View logs

@eonarheim eonarheim merged commit 582fbcc into main Apr 8, 2024
6 of 7 checks passed
@eonarheim eonarheim deleted the fix/2378 branch April 8, 2024 03:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chore Maintenance tasks, like upgrading libraries or release tasks
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants