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

Clean up after renaming the be channel to main. #115

Merged

Conversation

sortie
Copy link
Contributor

@sortie sortie commented Nov 13, 2023

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 :)

@@ -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
Copy link
Member

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?

Copy link
Contributor Author

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.

Copy link
Member

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
tool/sig.dart Outdated Show resolved Hide resolved
Co-authored-by: Jacob MacDonald <jakemac@google.com>
@devoncarew
Copy link
Member

@sortie - we should land this PR; we still need a changelog entry for the change.

Copy link
Member

@devoncarew devoncarew left a 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

@devoncarew devoncarew merged commit c1b2cdb into main Jan 9, 2024
27 checks passed
@devoncarew devoncarew deleted the sortie-clean-up-after-renaming-the-be-channel-to-main branch January 9, 2024 19:49
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.

None yet

3 participants