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

ci: use h3-nightly@latest for edge releases #1563

Merged
merged 5 commits into from
Aug 11, 2023
Merged

ci: use h3-nightly@latest for edge releases #1563

merged 5 commits into from
Aug 11, 2023

Conversation

pi0
Copy link
Member

@pi0 pi0 commented Aug 11, 2023

πŸ”— Linked issue

❓ Type of change

  • πŸ“– Documentation (updates to the documentation, readme, or JSdoc annotations)
  • 🐞 Bug fix (a non-breaking change that fixes an issue)
  • πŸ‘Œ Enhancement (improving an existing functionality like performance)
  • ✨ New feature (a non-breaking change that adds functionality)
  • 🧹 Chore (updates to the build process or auxiliary tools and libraries)
  • ⚠️ Breaking change (fix or feature that would cause existing functionality to change)

πŸ“š Description

πŸ“ Checklist

  • I have linked an issue or discussion.
  • I have updated the documentation accordingly.

@pi0 pi0 changed the title ci: use h3-nightly for edge releases ci: use h3-nightly for edge releases Aug 11, 2023
@pi0 pi0 requested a review from danielroe August 11, 2023 11:40
@codecov
Copy link

codecov bot commented Aug 11, 2023

Codecov Report

Merging #1563 (e0533dd) into main (75ddbf4) will not change coverage.
The diff coverage is n/a.

❗ Current head e0533dd differs from pull request most recent head ef87b52. Consider uploading reports for the commit ef87b52 to get more accurate results

@@           Coverage Diff           @@
##             main    #1563   +/-   ##
=======================================
  Coverage   76.49%   76.49%           
=======================================
  Files          73       73           
  Lines        7582     7582           
  Branches      752      752           
=======================================
  Hits         5800     5800           
  Misses       1781     1781           
  Partials        1        1           

@pi0 pi0 changed the title ci: use h3-nightly for edge releases ci: use h3-nightly@latest for edge releases Aug 11, 2023
@@ -117,6 +119,11 @@ async function main() {
`${pkg.data.version}-${date}.${commit}`
);
workspace.rename(pkg.data.name, pkg.data.name + "-edge");
pkg.updateDeps((dep) => {
if (nightlyPackages[dep.name]) {
dep.range = "npm:" + nightlyPackages[dep.name] + "@latest";
Copy link
Member

Choose a reason for hiding this comment

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

see my comment at nuxt/nuxt#22593 (comment).

I'm not sure latest is safe here. I'd recommend a caret + version hash.

Copy link
Member Author

Choose a reason for hiding this comment

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

Also see my comment there! I think it should be safer than pinned version to avoid hoisting issues on edge.

Copy link
Member Author

Choose a reason for hiding this comment

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

Reposting from nuxt/nuxt#22593 (comment), caret + hash is respected by package manager in a way that it won't look for any newer version + (sadly) h3-nightly@^1 doesn't works with packages published with - version.

@pi0
Copy link
Member Author

pi0 commented Aug 11, 2023

Merging to test end-to-end with the package manager behavior + coupled with nuxt edge.

I think if it is introducing too much trouble, we shall revert it and keep using h3 latest stable release. 1.8 was kinda exceptional release that taking too much rc(s).

@pi0 pi0 merged commit aa4a0af into main Aug 11, 2023
13 checks passed
@pi0 pi0 deleted the chore/ci-h3-nightly branch August 11, 2023 12:12
@pi0 pi0 mentioned this pull request Aug 21, 2023
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