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

black formatting #12797

Closed
wants to merge 1 commit into from
Closed

black formatting #12797

wants to merge 1 commit into from

Conversation

svlandeg
Copy link
Member

@svlandeg svlandeg commented Jul 6, 2023

Description

There was a strange github issue with syncing on PR #12679, it was green and I could merge it, then only after the merge it showed the test failure. Pretty strange. Anyway, adding the black formatting.

Types of change

format fix

Checklist

  • I confirm that I have the right to submit this contribution under the project's MIT license.
  • I ran the tests, and all new and existing tests passed.
  • My changes don't require a change to the documentation, or if they do, I've added all required information.

@svlandeg svlandeg added the meta Meta topics, e.g. repo organisation and issue management label Jul 6, 2023
@adrianeboyd
Copy link
Contributor

I noticed with GHA there's often a delay before it updates to the most recent commit, so sometimes it's still showing the green check from the previous run in the wrong spot.

But can we revert #12679 instead and move it to develop?

@svlandeg
Copy link
Member Author

svlandeg commented Jul 6, 2023

I noticed with GHA there's often a delay before it updates to the most recent commit, so sometimes it's still showing the green check from the previous run in the wrong spot.

Ah interesting, I hadn't seen that before. But this time I literally saw it updating the page as I was merging, even though it was showing the last commit be green before.

But can we revert #12679 instead and move it to develop?

We can definitely revert if you prefer, but why target develop? That PR shouldn't be breaking?

@svlandeg
Copy link
Member Author

svlandeg commented Jul 6, 2023

Reverted, closing in favour of #12798 (master vs develop still needs to be discussed though)

@svlandeg svlandeg closed this Jul 6, 2023
@svlandeg svlandeg deleted the fix/black branch July 6, 2023 15:07
@adrianeboyd
Copy link
Contributor

I thought we had already configured develop for v3.7?

I thought we had agreed not modify types on patch releases, except for possibly minor cases where we needed to widen a type?

@svlandeg
Copy link
Member Author

svlandeg commented Jul 6, 2023

I thought we had agreed not modify types on patch releases, except for possibly minor cases where we needed to widen a type?

Right - that's what I'm trying to understand, which change you see as modifying or breaking in that original PR.

@adrianeboyd
Copy link
Contributor

I think adding types where there weren't types before is narrowing and has the potential to possibly trigger new type checking errors for users?

Let me also comment on the new PR...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
meta Meta topics, e.g. repo organisation and issue management
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants