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

Backport PR #46558: CI: pre-commit autoupdate to fix CI #46563

Conversation

MarcoGorelli
Copy link
Member

@MarcoGorelli MarcoGorelli commented Mar 29, 2022

Backport PR #46558

@MarcoGorelli MarcoGorelli added the Code Style Code style, linting, code_checks label Mar 29, 2022
@simonjayhawkins
Copy link
Member

@MarcoGorelli There are a few CI PRs to manually backport. starting with #46543 (have opened #46559)

It maybe better to not merge this one yet. (makes autobackports more likely to succeed)

Out of curiosity, where were the conflicts that prevented the autobackport of #46558 succeeding

@MarcoGorelli
Copy link
Member Author

sure, will hold off on this one

they were conflicts with another PR which updated black version pins

@simonjayhawkins simonjayhawkins marked this pull request as draft March 29, 2022 14:11
@simonjayhawkins
Copy link
Member

Thanks. Have marked as draft and will do the backports in order, so this one after #46541 and #46540

@simonjayhawkins
Copy link
Member

they were conflicts with another PR which updated black version pins

I guess #46490 which we didn't backport. so probably autobackport won't suceed even after getting all the ci stuff in sync.

@simonjayhawkins simonjayhawkins marked this pull request as ready for review March 30, 2022 12:09
@simonjayhawkins simonjayhawkins added this to the 1.4.2 milestone Mar 30, 2022
@simonjayhawkins
Copy link
Member

code check are failing, I assume since we did not backport #45752

in #46558 (comment), I said ...

I assume will will get the same error on 1.4.x?

even though we are not seeing the same failures on 1.4.x

It maybe that since black on 1.4.x is pinned to 21.12b0 whereas on main it was 22.1.0 that we don't need the backport.

Alternatively, since the black changes should not affect code behavior, we could run the latest black as a commit here (I assume that would be easier than backporting #45752 and #46490)

@MarcoGorelli
Copy link
Member Author

MarcoGorelli commented Mar 30, 2022

Looks like it works fine in 1.4.x - maybe 21.12b0 isn't affected by the latest click release then?

I'd suggest to then either not backport black changes at all, or if click does cause issues, then just to pin that in additional_requirements for this hook

Or will not backporting black changes make other backports tricker?

@simonjayhawkins
Copy link
Member

Or will not backporting black changes make other backports tricker?

There we many files changed in #45752 so there is perhaps a greater risk that a manual backport is required in the future due to a conflict.

I'm +1 on updating the code on 1.4.x to the latest black (although IIRC we have not tended to do this in the past)

@MarcoGorelli
Copy link
Member Author

sure, I've done this in #46576, will close this one

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Code Style Code style, linting, code_checks
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants