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

feat: deno upgrade --rc #24905

Merged
merged 27 commits into from
Aug 12, 2024
Merged

feat: deno upgrade --rc #24905

merged 27 commits into from
Aug 12, 2024

Conversation

bartlomieju
Copy link
Member

Flag name open to bike-shedding.

@bartlomieju bartlomieju marked this pull request as draft August 6, 2024 07:16
cli/Cargo.toml Outdated Show resolved Hide resolved
cli/tools/upgrade.rs Outdated Show resolved Hide resolved
@bartlomieju bartlomieju added this to the 1.46 milestone Aug 7, 2024
@bartlomieju bartlomieju marked this pull request as ready for review August 9, 2024 11:58
cli/tools/upgrade.rs Outdated Show resolved Hide resolved
release-2-rc.txt Outdated
@@ -0,0 +1,2 @@
0b8f8d0f60ad0ff501860bead0fd333634560d30 v1.46.0-rc.0
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove, add a GH actions script that appends to a file like this in GCP bucket

foo.js Outdated Show resolved Hide resolved
))
);
if !upgrade_flags.canary {
print_release_notes(version::deno(), &install_version);
if !requested_version.is_canary() {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Decide if we're gonna do RC blog post/and or link to notes

cli/tools/upgrade.rs Outdated Show resolved Hide resolved
cli/tools/upgrade.rs Outdated Show resolved Hide resolved
cli/tools/upgrade.rs Outdated Show resolved Hide resolved
cli/tools/upgrade.rs Outdated Show resolved Hide resolved
@@ -864,13 +1035,14 @@ struct CheckVersionFile {
pub last_checked: chrono::DateTime<chrono::Utc>,
pub current_version: String,
pub latest_version: String,
pub current_release_channel: ReleaseChannel,
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I opted to store this info in the latest.txt file so we don't have to hit remote server to display a banner. Works quite well 👍

@@ -725,6 +890,7 @@ fn get_latest_version_url(
ReleaseChannel::Canary => {
Cow::Owned(format!("canary-{target_tuple}-latest.txt"))
}
ReleaseChannel::Rc => Cow::Borrowed("release-2-rc.txt"),
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this PR provides a generic solution to RCs it's probably best to rename it to release-rc.txt

@ry
Copy link
Member

ry commented Aug 11, 2024

Should we also add --lts already as well?

@bartlomieju
Copy link
Member Author

Should we also add --lts already as well?

We can, but I think I want to try this mechanism first for a few days. I can do it once we're happy with it.

cli/args/flags.rs Outdated Show resolved Hide resolved
Copy link
Member

@dsherret dsherret left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM once the flag name is changed to use a dash.


let is_current_exe_an_rc = rc_versions
.iter()
.any(|(hash, _)| hash == version::GIT_COMMIT_HASH);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is realy hacky, but ok for the first pass.

@bartlomieju bartlomieju enabled auto-merge (squash) August 12, 2024 18:02
@bartlomieju bartlomieju merged commit 3c70b94 into denoland:main Aug 12, 2024
17 checks passed
@bartlomieju bartlomieju deleted the deno_upgrade_rc branch August 12, 2024 21:02
bartlomieju added a commit to bartlomieju/deno that referenced this pull request Aug 12, 2024
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.

4 participants