-
Notifications
You must be signed in to change notification settings - Fork 46
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
fix(woodpecker): update woodpecker env vars #322
Conversation
i'm not a user of woodpecker, so i could use some help understanding the impact of this change. are the old values no longer available on new versions of woodpecker servers? could some folks still be using older versions of a woodpecker server where these new values are not available (meaning this would be a breaking change)? any other impacts that we should communicate to consumers? |
Not yet, so all currently released versions support both. They're deprecated and will be removed in next versions.
This is also unlikely because they're available for some time already and all supported versions support these new env vars. If your woodpecker server is on an unsupported old version this could be possible, but otherwise not. |
Hey @travi, can we go on with this? |
Hi @travi, the situation has changed. Woodpecker released a new version that must use these new env vars (all other versions still using the old ones are not supported so they shouldn't be used anymore). I'd like to get this done. Merge this in the next days, or I'll close it, but then this package won't work with woodpecker anymore. I don't understand why reviewing a PR changing just 10 lines takes over 3 weeks to get merged. (And I can see in you GitHub activity that you're not completely inactive.) |
@travi I'm closing this now. |
what positive outcome does this bring? by closing your PR, you have no chance of it being merged. i'm not going to apologize for the timeline. i would normally be more apologetic for not communicating my plan more openly, but behavior like this makes me far less concerned about your goals.
this is a breaking change. even if it is unlikely to break many folks in reality, it is still a breaking change and would be released as a major version. semantic-release is the primary consumer of this package and that type of breaking change is also a breaking change for semantic-release, so would not be merged into semantic-release until we are ready to release a new major there. during this time, i've been coordinating other pieces that would go into a major release of semantic-release, so i could make the timing of merging this have the smoothest upgrade path possible. i had hoped to get a beta release of the next major started this last friday, but didnt manage to get to it. part of that plan was to get this merged and released as part of that.
so would i, but the state of OSS is that accepting contributions and considering impacts to our consumers takes time and effort.
yes, i do have other priorities than yours if you want this included in the next major, feel free to re-open, but you need to be patient. if that major version is released without this change, releasing this change will likely be delayed significantly more |
That's fine, but this was open a month now.
I reopened, but I don't really see it's a breaking change. You shouldn't use unsupported versions of a software, so it will be broken on newer woodpecker versions instead, and merging this shouldn't break anyone that uses a supported version. |
@qwerty287 i'm ready to get this merged in support of semantic-release/semantic-release#3111, but it looks like there are test failures. could you look into resolving those failures? |
🎉 This PR is included in version 11.0.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
if you are using env-ci through semantic-release, this is now available there as a beta release: https://github.com/semantic-release/semantic-release/releases/tag/v23.0.0-beta.3 |
Hi!
Woodpecker updated some env var names (see https://woodpecker-ci.org/docs/next/migrations#100). This PR updates them to the latest ones.