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: Linting #1417

Merged
merged 34 commits into from
Sep 2, 2021
Merged

chore: Linting #1417

merged 34 commits into from
Sep 2, 2021

Conversation

maxwellmattryan
Copy link
Contributor

@maxwellmattryan maxwellmattryan commented Aug 18, 2021

Description of change

  • Adds ESLint and Prettier as root-level dependencies (use yarn lint and yarn format[-check])
  • Adds rustfmt for linting *.rs files
    • Currently only in the CI workflow
  • Adds GitHub Action for running a linting job on any PR and pushes to develop
  • Adds pre-commit Git hook via Husky that checks format and then lints
  • Changes .github/CONTRIBUTING.md with new linting step and updating CI link
  • Re-formats code with yarn format in root dir (NOTE: linting errors are everywhere so they will be in a separate PR)

NOTE: .svelte files do not mix well with Prettier for some reason, so they are omitted. It is mainly just .ts files that have the formatting.

Links to any relevant issues

None

Type of change

  • Chore (refactoring, build scripts or anything else that isn't user-facing)

How the change has been tested

Tested on:

  • Ubuntu (20.04)
  • Windows 10 (19042.1110)

Change checklist

  • I have followed the contribution guidelines for this project
  • I have performed a self-review of my own code
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

@maxwellmattryan maxwellmattryan added the type:chore House-keeping etc. label Aug 18, 2021
@maxwellmattryan maxwellmattryan marked this pull request as ready for review August 18, 2021 17:51
Copy link
Member

@rajivshah3 rajivshah3 left a comment

Choose a reason for hiding this comment

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

Nice!

@maxwellmattryan maxwellmattryan added the scope:ci Affects CI actions label Aug 20, 2021
@maxwellmattryan maxwellmattryan mentioned this pull request Aug 20, 2021
7 tasks
.github/workflows/firefly-ci.yml Outdated Show resolved Hide resolved
.github/workflows/firefly-ci.yml Outdated Show resolved Hide resolved
@maxwellmattryan maxwellmattryan requested review from rajivshah3 and removed request for cvarley100 August 31, 2021 19:27
Copy link
Member

@rajivshah3 rajivshah3 left a comment

Choose a reason for hiding this comment

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

Thanks for adding rustfmt! Just a few suggestions

.github/CONTRIBUTING.md Outdated Show resolved Hide resolved
.github/workflows/ci.lint.yml Outdated Show resolved Hide resolved
Copy link
Member

@rajivshah3 rajivshah3 left a comment

Choose a reason for hiding this comment

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

Great work!

@maxwellmattryan maxwellmattryan merged commit 85a966b into develop Sep 2, 2021
@maxwellmattryan maxwellmattryan deleted the chore/linting branch September 2, 2021 15:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
scope:ci Affects CI actions type:chore House-keeping etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants