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: Add Husky to project #755

Merged
merged 2 commits into from
Nov 15, 2021
Merged

chore: Add Husky to project #755

merged 2 commits into from
Nov 15, 2021

Conversation

achou11
Copy link
Member

@achou11 achou11 commented Sep 30, 2021

Notes:

@gmaclennan
Copy link
Member

Just wondering if lint-staged and pretty-quick --staged might conflict with each other? They both do the same thing don't they? (i.e. just act on staged files) but pretty-quick I think has its own logic for this.

@achou11
Copy link
Member Author

achou11 commented Sep 30, 2021

Just wondering if lint-staged and pretty-quick --staged might conflict with each other? They both do the same thing don't they? (i.e. just act on staged files) but pretty-quick I think has its own logic for this.

Yeah i was somewhat confused because I initially tried with just pretty-quick (without the --staged) but that didn't actually commit the changes. Had to actually add the flag to have it commit changes.

Alternatively, we could forgo using lint-staged if we're thinking that formatting is the only thing we really want to do with the staged files 🤷‍♂️ would kind of differ from the desktop PR but i don't have strong attachments to it

@ErikSin
Copy link
Contributor

ErikSin commented Oct 11, 2021

Just wondering if lint-staged and pretty-quick --staged might conflict with each other? They both do the same thing don't they? (i.e. just act on staged files) but pretty-quick I think has its own logic for this.

Yeah i was somewhat confused because I initially tried with just pretty-quick (without the --staged) but that didn't actually commit the changes. Had to actually add the flag to have it commit changes.

Alternatively, we could forgo using lint-staged if we're thinking that formatting is the only thing we really want to do with the staged files 🤷‍♂️ would kind of differ from the desktop PR but i don't have strong attachments to it

I think having both is unnecessary And pretty-quick is doing what we want it to do already. I think for ease, lets get rid of lint-stage

Copy link
Contributor

@ErikSin ErikSin left a comment

Choose a reason for hiding this comment

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

Lets get rid of lint-stages

@achou11 achou11 force-pushed the ac/add-husky branch 2 times, most recently from de3debe to db4ba4c Compare October 20, 2021 17:00
@achou11 achou11 requested a review from ErikSin November 12, 2021 20:30
@ErikSin
Copy link
Contributor

ErikSin commented Nov 15, 2021

Feel free to merge after rebase

@achou11 achou11 merged commit 1aa1749 into develop Nov 15, 2021
@achou11 achou11 deleted the ac/add-husky branch November 15, 2021 15:58
gmaclennan added a commit that referenced this pull request Nov 23, 2021
…ject-invite-server

* commit '1aa1749aaf6e5a439234e9fedf7ff28d05cd0edb':
  chore: Add Husky to project (#755)
  chore: Fix small typo in translation description (#868)
  feat: Show alert after successfully importing config (#864)
  chore(release): Prepare release 5.3.2
  fix: Add missing Portuguese and Spanish translations
  chore(release): Prepare release 5.3.1
  fix: Do not surface Practice Mode in config details in settings (#852)
  fix: Enable navigation to Experiments screen (#853)
  fix: Do not surface Practice Mode in config details in settings (#852)
  fix: Enable navigation to Experiments screen (#853)
  feat: Add Create Project flow (#846)
  chore(deps): bump ansi-regex from 5.0.0 to 5.0.1 in /src/backend (#753)
  feature: Add new translation strings (en) (#754)
  chore: Patch nodejs-mobile to specify NDK version (#757)
  Update contributing docs for NDK fix
  chore(release): Prepare release 5.3.0
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.

3 participants