ci: add ruff for print checks by diivi · Pull Request #1666 · borgbase/vorta

diivi · 2023-03-21T09:08:01Z

Description

Recently committed a print statement, which reminded me of #1612 (comment)

Screenshots (if appropriate):

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

I have read the CONTRIBUTING guide.
My code follows the code style of this project.
My change requires a change to the documentation.
I have updated the documentation accordingly.
I have added tests to cover my changes.
All new and existing tests passed.

I provide my contribution under the terms of the license of this repository and I affirm the Developer Certificate of Origin.

Hofer-Julian

Thanks, looks good in general.

m3nu · 2023-04-05T11:32:20Z

What's missing to move this forward?

diivi · 2023-04-05T11:40:23Z

What's missing to move this forward?

Just a few minor changes:

keeping isort and not using ruff's isort.
exclude package in tests.

real-yfprojects · 2023-04-05T16:14:35Z

It would be great if you could seperate your changes from the autogenerated ones. This allow excluding the latter from git blame.

diivi · 2023-04-05T19:29:17Z

seperate your changes from the autogenerated ones.

You mean make all config changes, skip pre-commit and push directly?
How will the sorting changes be made then?

This allow excluding the latter from git blame.

Oh, shouldn't git blame just blame me for the empty lines, and the other stuff remains the same?

real-yfprojects · 2023-04-06T08:07:09Z

You mean make all config changes, skip pre-commit and push directly?

Maybe run pre-commit on the config changes before committing those.

How will the sorting changes be made then?

Are you talking about the changes by rust?

shouldn't git blame just blame me for the empty lines, and the other stuff remains the same?

I didn't look into the exact changes prior to writing my comment. This is just the general process of changing autoformatter configs. However rust didn't only add new lines I think.

diivi · 2023-04-06T11:54:35Z

rust

ruff 😅

So you mean I should make 2 commits right - one with my changes and the other one with ruff's changes?

real-yfprojects · 2023-04-06T12:55:21Z

rust

ruff 😅

So you mean I should make 2 commits right - one with my changes and the other one with ruff's changes?

Yes

diivi · 2023-04-06T17:44:01Z

Config changes - 5d8073b.
I'll handle the merge conflicts after this is checked.

real-yfprojects · 2023-04-08T08:17:48Z

I'll handle the merge conflicts after this is checked.

Looking good. After resolving merge conflicts (I think rebasing makes the most sense) I would squash the first two commits. Then a commit can be created that adds run formatter to .git-blame-ignore-revs.

diivi · 2023-04-11T04:44:25Z

Tried doing that, but I think I messed something up here - https://github.com/borgbase/vorta/compare/master...diivi:test-ruff-commits?expand=1.
I'll try to redo the changes in order.

real-yfprojects · 2023-04-11T07:03:29Z

Tried doing that, but I think I messed something up

Make sure you local master is up-to-date with upstream/master.
Checkout this branch
git rebase -i master
In the file opened select to squash ca90d7d373e4dfdd14ce2b10017516937bf43c9c and to drop a5d433fb252ddb8b0c97b803fc4eba58dd3e161d
Save and close the file
Rerun the automatic formatters and commit the changes to the code base
Add the hash of the last commit to .git-blame-ignore-revs and commit it.

diivi · 2023-04-11T11:22:21Z

Done, thanks!

m3nu · 2023-04-11T17:07:00Z

This changes almost all the files by changing some formatting. Please only add the actual changes. If formatting settings change, this can be a separate PR. Else I can't really combine this with the qt6 PR, which also touches many files.

diivi · 2023-04-11T17:13:47Z

So only d451c1d

real-yfprojects · 2023-04-11T19:15:43Z

Merging this without the commit containing the formatting changes will break ci afaik.

m3nu · 2023-04-11T19:34:41Z

Wasn't this to find print statements when it started? 😀

And wasn't Black doing the formatting? Can't we just keep the same settings?

Anyways, if everything needs to be reformatted again, I suggest to just add your config changes and apply the formatting later when there aren't many pending PRs. Better to rerun the formatter than deal with all the conflicts.

diivi · 2023-04-12T03:43:28Z

Wasn't this to find print statements when it started? grinning

Yeah, now it's about replacing isort with ruff too 😅 #1666 (comment)

m3nu · 2023-04-12T09:44:34Z

Understood. Then I suggest this:

Please all help testing PyQt6 Upgrade #1685 so we can merge it today or tomorrow. This will start a new major version, since a major dependency changes.
For this PR, please remove the formatting commits and only keep the commit that changes the tooling config.
After Qt6 is merged, you can add the formatting changes without worrying about conflicts.
Since this is a new major version, one could consider expanding the use of Ruff and replacing some more tools, if you like.

m3nu · 2023-04-17T10:22:17Z

#1685 is merged. So good to proceed here and add the formatting changes needed.

real-yfprojects · 2023-04-29T10:45:30Z

Does everyone approve this PR? Then I'll take care of merging this correctly.

m3nu · 2023-04-29T11:40:45Z

Good to go! Formatting change separate, if possible. 🙏

real-yfprojects · 2023-04-29T16:35:17Z

Formatting change separate

I am planning on fast forwarding master with 3 commits corresponding to the commits of this PR.

real-yfprojects · 2023-05-01T08:08:12Z

 	flake8
-	isort --check-only .
 	black --check .
+	ruff check . --fix


Why --fix? This is intended as a check only. That's why black is run with --check.

real-yfprojects · 2023-05-01T08:11:15Z


+  - repo: https://github.com/charliermarsh/ruff-pre-commit
+    rev: 'v0.0.257'
+    hooks:


I'd prefer running the pre-commit hook with --fix.

Adds ruff replacing isort. Ruff comes with all flake8 rules and additional rules for print statements. * .github/workflows/test.yml : Replace isort with ruff in comment * .editorconfig : Update `yml` config to apply to all yaml files. * Makefile (lint): Run ruff, remove isort * .pre-commit-config.yaml : Remove isort, run ruff * pyproject.toml : Configure ruff. Remove isort config. * requirements.d/dev.txt : Add ruff, remove isort * setup.cfg : Extend flake8 file ignore * src/vorta/__main__.py : Add *noqa* for print statement.

Includes all changes by `pre-commit --all-files` including the changes introduced by ruff.

* .git-blame-ignore-revs

Hofer-Julian reviewed Mar 21, 2023

View reviewed changes

Comment thread pyproject.toml Outdated

Comment thread .github/workflows/test.yml

Hofer-Julian reviewed Mar 21, 2023

View reviewed changes

Comment thread Makefile Outdated

real-yfprojects reviewed Mar 21, 2023

View reviewed changes

Comment thread package/fix_app_qt_folder_names_for_codesign.py

diivi force-pushed the ci/add-ruff-for-print-detection branch from a4e9b92 to 5d8073b Compare April 6, 2023 17:40

diivi force-pushed the ci/add-ruff-for-print-detection branch from 9c814f9 to a7d6ed0 Compare April 11, 2023 11:21

diivi force-pushed the ci/add-ruff-for-print-detection branch from a7d6ed0 to d451c1d Compare April 11, 2023 17:24

real-yfprojects reviewed May 1, 2023

View reviewed changes

real-yfprojects added 2 commits May 1, 2023 10:28

Run pre-commit (with newly added ruff) on code base.

24e1dd5

Includes all changes by `pre-commit --all-files` including the changes introduced by ruff.

Add re-format with ruff to .git-blame-ignore.revs.

82270ad

* .git-blame-ignore-revs

real-yfprojects force-pushed the ci/add-ruff-for-print-detection branch from e333c65 to 82270ad Compare May 1, 2023 08:31

real-yfprojects approved these changes May 9, 2023

View reviewed changes

real-yfprojects merged commit 82270ad into borgbase:master May 9, 2023

Conversation

diivi commented Mar 21, 2023

Description

Screenshots (if appropriate):

Types of changes

Checklist:

Uh oh!

Hofer-Julian left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

m3nu commented Apr 5, 2023

Uh oh!

diivi commented Apr 5, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

real-yfprojects commented Apr 5, 2023

Uh oh!

diivi commented Apr 5, 2023

Uh oh!

real-yfprojects commented Apr 6, 2023

Uh oh!

diivi commented Apr 6, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

real-yfprojects commented Apr 6, 2023

Uh oh!

diivi commented Apr 6, 2023

Uh oh!

real-yfprojects commented Apr 8, 2023

Uh oh!

diivi commented Apr 11, 2023

Uh oh!

real-yfprojects commented Apr 11, 2023

Uh oh!

diivi commented Apr 11, 2023

Uh oh!

m3nu commented Apr 11, 2023

Uh oh!

diivi commented Apr 11, 2023

Uh oh!

real-yfprojects commented Apr 11, 2023

Uh oh!

m3nu commented Apr 11, 2023

Uh oh!

diivi commented Apr 12, 2023

Uh oh!

m3nu commented Apr 12, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

m3nu commented Apr 17, 2023

Uh oh!

real-yfprojects commented Apr 29, 2023

Uh oh!

m3nu commented Apr 29, 2023

Uh oh!

real-yfprojects commented Apr 29, 2023

Uh oh!

real-yfprojects May 1, 2023

Choose a reason for hiding this comment

Uh oh!

real-yfprojects May 1, 2023

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

diivi commented Apr 5, 2023 •

edited

Loading

diivi commented Apr 6, 2023 •

edited

Loading

m3nu commented Apr 12, 2023 •

edited

Loading