Skip to content

build: use pre-commits to lint the lib#237

Merged
mariobuikhuizen merged 8 commits intowidgetti:masterfrom
pyvuetify:pre-commit
Mar 6, 2023
Merged

build: use pre-commits to lint the lib#237
mariobuikhuizen merged 8 commits intowidgetti:masterfrom
pyvuetify:pre-commit

Conversation

@12rambau
Copy link
Copy Markdown
Contributor

@12rambau 12rambau commented Jan 7, 2023

I added a .pre-commit config file to lint the lib using the following tools:

  • black
  • flake8
  • prettier
  • commitizen
  • isort

I also took the liberty to add 2 badges, and some configuration in the setup.cfg

Fix #191

@maartenbreddels
Copy link
Copy Markdown
Collaborator

Nice, I’m in favour of this, but maybe replace isort and black with ruff?

@12rambau
Copy link
Copy Markdown
Contributor Author

12rambau commented Jan 7, 2023

I wasn't aware of ruff, let me update the config and run it again

@12rambau
Copy link
Copy Markdown
Contributor Author

12rambau commented Jan 7, 2023

@maartenbreddels on the ruff readme they suggest to replace flake8 and isort but not black:

As a project, Ruff is designed to be used alongside Black and, as such, will defer implementing stylistic lint rules that are obviated by autoformatting.

should I go in this direction ?

@maartenbreddels
Copy link
Copy Markdown
Collaborator

Let’s see what Mario thinks

@mariobuikhuizen
Copy link
Copy Markdown
Collaborator

Looks good to me! Thanks!

Regarding ruff, I don't know much about it, but if the authors recommend not replacing black, I'd go with that.

Copy link
Copy Markdown
Collaborator

@mariobuikhuizen mariobuikhuizen 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 this contribution @12rambau!

Could you remove the package-lock files? (the changes are probably caused by using a different version of nodejs)

After that, can you squash the commits? I think this change can be one commit.

@12rambau
Copy link
Copy Markdown
Contributor Author

12rambau commented Feb 28, 2023

I rollback to the previous version of the package-lock files and removed ".vscode" from the .gitignore.

I didn't squash anything from my side as I don't think the intermediate commits are meaningful. Could you instead squash it directly using the merge button? side advantage, you'll be able to select the commit message (I'm never sure if I'm using the appropriate conventional commit classifier)

PS: please do that for any of my PR (we are doing the same in pydata-sphinx-theme and that gives nice changelog and easy to follow commit history).

@mariobuikhuizen
Copy link
Copy Markdown
Collaborator

Yeah, I can do that 👍 , but I thought you might want to write the commit message and have the commit on your name.

@12rambau
Copy link
Copy Markdown
Contributor Author

12rambau commented Feb 28, 2023

the commit will still be on my name no worries, and you can use the title of the PR (I've chosen it to be used as such)

@mariobuikhuizen mariobuikhuizen merged commit 36ddc38 into widgetti:master Mar 6, 2023
@12rambau 12rambau deleted the pre-commit branch March 6, 2023 11:23
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.

use the black formatter ?

3 participants