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

[enhancement]: Use black & isort for code formatting. #2522

Closed
1 task done
dsully opened this issue Feb 5, 2023 · 9 comments
Closed
1 task done

[enhancement]: Use black & isort for code formatting. #2522

dsully opened this issue Feb 5, 2023 · 9 comments
Assignees
Labels
CI-CD Continuous integration / Continuous delivery enhancement New feature or request Inactive Issue

Comments

@dsully
Copy link
Contributor

dsully commented Feb 5, 2023

Is there an existing issue for this?

  • I have searched the existing issues

Contact Details

dsully#8817, @dsully@mastodon.social

What should this feature add?

I'm proposing to use black and isort via GitHub Actions to format this code base so that contributions will be consistent.

Alternatives

No response

Aditional Content

No response

@dsully dsully added the enhancement New feature or request label Feb 5, 2023
@ebr ebr added the CI-CD Continuous integration / Continuous delivery label Feb 5, 2023
@ebr
Copy link
Member

ebr commented Feb 5, 2023

Great suggestion, thank you. we can add black to the dev toolchain and have it run in the workflows at least in the --check-only mode, but i'm afraid allowing CI to commit formatted/linted changes will cause divergence between local and remote branches. For many contributors it's a high barrier to deal with, or simply too frustrating. Do you have any ideas around introducing this in a non-disruptive manner?

@dsully
Copy link
Contributor Author

dsully commented Feb 5, 2023

Are you referring to remote branches that haven't been merged yet, or something else?

Using --check in a GHA Workflow + a one time formatting. Unless you're worried about the rebasing for existing branches in this repo?

Alternatively (or perhaps in addition), using https://pre-commit.com might work well, since git doesn't have remote pre-commit hooks.

@psychedelicious
Copy link
Collaborator

psychedelicious commented Feb 6, 2023

FWIW, the UI will be using a husky to format and lint via pre-commit hook. It's being used in my nodes migration branch for now. This is now in main.

@lstein
Copy link
Collaborator

lstein commented Feb 8, 2023

I discussed this with @mauwii and he’s going to put in a pre-commit hook to accomplish this.

@dsully
Copy link
Contributor Author

dsully commented Feb 8, 2023

Fantastic. Let me know if I can help at all.

@mauwii
Copy link
Contributor

mauwii commented Feb 8, 2023

I did kind of a PoC here: https://github.com/mauwii/PyPatchMatch

Need some further discussions about how to integrate it into InvokeAI. Are you on our Discord @dsully?

@dsully
Copy link
Contributor Author

dsully commented Feb 8, 2023

I am, but haven't had really any time to look at it between day job and teenagers. dsully#8817

@mauwii
Copy link
Contributor

mauwii commented Feb 8, 2023

Ok, np then 😅

@github-actions
Copy link

There has been no activity in this issue for 14 days. If this issue is still being experienced, please reply with an updated confirmation that the issue is still being experienced with the latest release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI-CD Continuous integration / Continuous delivery enhancement New feature or request Inactive Issue
Projects
None yet
Development

No branches or pull requests

5 participants