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

Add pre-commit-config #298

Merged
merged 2 commits into from
Oct 24, 2022
Merged

Add pre-commit-config #298

merged 2 commits into from
Oct 24, 2022

Conversation

redphix
Copy link
Contributor

@redphix redphix commented Oct 21, 2022

What does this PR do?

Adds a pre-commit config to run linters/formaters before committing

Steps every contributor must follow before working:

  1. make sure all the dependencies, include dev dependencies for python and node are installed
  2. run pip install pre-commit
  3. run pre-commit install
  4. write awesome features, stage and commit your changes. The linters and formatters will run according to what you touch. If it's just a lint error (flake8) then fix it and stage the changes again and commit it.

Checklist before merging

  • If it's a frontend feature, I have ran prettier cd frontend; npm run format. If it's a mobile app feature I ran cd mobile; npm run format.

@redphix redphix marked this pull request as ready for review October 21, 2022 07:33
@@ -734,7 +734,11 @@ def update_invoice(cls, order, user, invoice):
if not order.taker_bond:
return False, {"bad_request": "Wait for your order to be taken."}
if (
not ( order.taker_bond.status == order.maker_bond.status == LNPayment.Status.LOCKED)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed the changes introduced by that 'annoy black' PR in this file

Copy link
Collaborator

@Reckless-Satoshi Reckless-Satoshi left a comment

Choose a reason for hiding this comment

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

Very cool!

We probably should add those 4 steps at the top of setup.md as they apply to all contributors.

Maybe prettier can be fixed? I do not manage to trigger it. Tried changing the files: key too to no success.

- merge-commit
language: system
files: ^frontend/
types: [javascript, jsx, ts, tsx, css, markdown, json] # uses https://github.com/pre-commit/identify
Copy link
Collaborator

Choose a reason for hiding this comment

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

First time using pre-commit pip package. This is really powerful!

It seems it is not running npm run format when I add some style mistake on the /frontend

prettier-frontend....................................(no files to check) Skipped

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ahh, was a mistake with the types key. Changed it to types_or

@Reckless-Satoshi
Copy link
Collaborator

Awesome, merging! 🚀

We did not have a rewarded task created for this PR beforehand. But it's actually pretty important and will make or life easier. Let's add a few sats to the positive Karma, please submit an invoice for 100K Sats from a proxy wallet with a long expiration time.

@Reckless-Satoshi Reckless-Satoshi merged commit c426d11 into RoboSats:main Oct 24, 2022
@redphix
Copy link
Contributor Author

redphix commented Oct 25, 2022

@Reckless-Satoshi Thank you ❤️ 🤗

lnbc1m1p340jk0pp5pfj23dt0mwpscx2kzheqg6pass6sfa6jytegxtak8gptlzmg367sdqqcqzpgxqyd9uqsp52k2mkt4gqed6uwm529ytwvx30t2accpjf0et64gx708cd4f5p9ns9qyyssqpmg6wc63f4sjd0qw4sl5xtapg3qdj62t2tu9ja9j0y60talme5e9s0nzn2xct6e9n47u50wtmlsh4j5mvgdswv4pc5e9eqcxhdu7kwgqrlytu4

@Reckless-Satoshi
Copy link
Collaborator

@Reckless-Satoshi Thank you heart hugs

lnbc1m1p340jk0pp5pfj23dt0mwpscx2kzheqg6pass6sfa6jytegxtak8gptlzmg367sdqqcqzpgxqyd9uqsp52k2mkt4gqed6uwm529ytwvx30t2accpjf0et64gx708cd4f5p9ns9qyyssqpmg6wc63f4sjd0qw4sl5xtapg3qdj62t2tu9ja9j0y60talme5e9s0nzn2xct6e9n47u50wtmlsh4j5mvgdswv4pc5e9eqcxhdu7kwgqrlytu4

fbe7afbb121b363e7f12fb2ed445116216698ef1a4dfb1be4e54c1f81cf9e5b4

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