-
Notifications
You must be signed in to change notification settings - Fork 38
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
Clean up after renaming the be channel to main. #115
Clean up after renaming the be channel to main. #115
Conversation
Bug: b/299435467
@@ -71,14 +71,7 @@ Future<void> _impl(List<String> args) async { | |||
channel = sdk; | |||
version = raw ? 'latest' : (await latestPublishedVersion(channel, flavor)); | |||
} else if (sdk == 'main') { | |||
// Check for `main` first and fall back to `be`. This handles the channel |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We'll want a changelog entry for this, and will need to bump the action's version as well.
Thinking through how users will see this change:
- old versions of the action, testing against old sdks, will work
- new versions of the action (after this change), testing against old sdks (2.19, 3.0, ...) will fail
- old versions of the action, testing against newer sdks, will fail
- new versions of the action, testing against new sdks, will work
Is this change essentially dropping support for sdks before some version? Or, just for main
channel sdks before some version? We probably don't need to support many previous versions for those sdks.
Can you update the changelog w/ an entry, and in that entry mention for which sdk version we first started writing artifacts into the main
bucket?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The main channel was only ever 'supported' for latest. There is no support in here for pinning a particular hash or an older SDK version.
Anyone that uses the old action before 1.5.0 (or what it was) will now be stuck on that last be release as of today, and anyone on that version or newer will use the main channel as of today (the compat logic I'm removing here automatically switched them together).
I'll add some changelog stuff and bump the version, this was just to get started while it was on my mind :)
Note that I am starting to feel we really should drop support for the raw flavor. The main channel builds is not a supported product in any way and the binaries aren't signed. I may even decide to store these artifacts elsewhere in the future to guard the release bucket even more closely. I'll also want to discontinue the whole raw/signed flavor concept, since people should not really be poking in there and I need to change the layout for unrelated reasons, and it'll massively simplify this action. But that's a discussion for another day.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Gotcha, thanks for the clarification re: how breaking this is; ~sounds like this will not be a breaking change. I think either a patch version rev or a minor version would be appropriate.
Note that I am starting to feel we really should drop support for the raw flavor.
I think the simplifications to this action would be welcome. We probably want to open an issue on this repo in order to discuss, get feedback, and help any user that might be impacted plan alternatives.
recompile the javascript code; normalize sig generation
Co-authored-by: Jacob MacDonald <[email protected]>
@sortie - we should land this PR; we still need a changelog entry for the change. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm; landing - will update the changelog separately
Bug: b/299435467
Let's wait a couple days before landing this one, just to make sure the channel rename sticks. I could use your help to recompile to javascript tho, like last time :)