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

branch 2.14 version of #4918 #4919

Draft
wants to merge 3 commits into
base: 2.14
Choose a base branch
from
Draft

Conversation

pjfanning
Copy link
Member

seems to show that the issue in #4917 / #4918 is new-ish

@pjfanning pjfanning marked this pull request as draft January 22, 2025 19:07
@cowtowncoder
Copy link
Member

@pjfanning Ah ok so this is where we get the tests.

I can merge into 2.14 and forward, although typically we don't really do PRs against un-maintained branches.
But I guess in this case there's also the question of how to coordinate fix vs test that fails for ... 2.18+ only?

@pjfanning
Copy link
Member Author

Starts failing in 2.15 - 2.14 is the last branch where it works.

@pjfanning pjfanning marked this pull request as ready for review January 22, 2025 22:49
@cowtowncoder
Copy link
Member

Starts failing in 2.15 - 2.14 is the last branch where it works.

Ok. So I think test should:

  1. Be disabled in intermediate versions where it'd fail (I don't see much point in fixing 2.14 as there's not going to be releases)
  2. Only merged in for version being fixed (2.17 or 2.18; maybe it's safe enough to go in 2.17... or possibly 2.16)
  3. Or if we really want to, be merged in 2.14 as passing and then add fix in 2.15 along with merged test

I am leaning towards (2) but can go with other options as well. While nasty bug, I don't think I'll be tempted to release 2.14 - 2.16 any more (2.16 is nominally open until 2.19.0)

@pjfanning pjfanning marked this pull request as draft January 23, 2025 11:00
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.

2 participants