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

Added semver checks #3173

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Added semver checks #3173

wants to merge 3 commits into from

Conversation

Razican
Copy link
Member

@Razican Razican commented Jul 25, 2023

This Pull Request adds SemVer checks to our CI. This comes under the idea of releasing more often, and trying to release semver-compatible releases if possible.

It changes the following:

  • Changes the current version to 0.17.1-dev, while we develop the new version
  • Uses cargo-semver-checks to make sure we are semver-compatible.

@Razican Razican added the test Issues and PRs related to the tests. label Jul 25, 2023
@Razican Razican added this to the v0.18.0 milestone Jul 25, 2023
@Razican Razican requested a review from a team July 25, 2023 18:38
@github-actions
Copy link

github-actions bot commented Jul 25, 2023

Test262 conformance changes

Test result main count PR count difference
Total 95,609 95,609 0
Passed 75,926 75,926 0
Ignored 18,924 18,924 0
Failed 759 759 0
Panics 0 0 0
Conformance 79.41% 79.41% 0.00%

@codecov
Copy link

codecov bot commented Jul 25, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (cec9892) 45.64% compared to head (040e51a) 45.45%.
Report is 5 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3173      +/-   ##
==========================================
- Coverage   45.64%   45.45%   -0.19%     
==========================================
  Files         483      483              
  Lines       49517    49743     +226     
==========================================
+ Hits        22600    22613      +13     
- Misses      26917    27130     +213     

see 7 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@nekevss nekevss requested a review from a team July 26, 2023 16:05
@jedel1043
Copy link
Member

jedel1043 commented Jul 26, 2023

Seeing that this modifies our CI, can we also repair our release action? I'm pretty sure we removed the rust publish command for release fixes

@Razican
Copy link
Member Author

Razican commented Jul 27, 2023

Seeing that this modifies our CI, can we also repair our release action? I'm pretty sure we removed the rust publish command for release fixes

Done :)

@nekevss
Copy link
Member

nekevss commented Jul 27, 2023

Oh, the version should be 0.18.0-dev instead of 0.17.1-dev, right?

@Razican
Copy link
Member Author

Razican commented Jul 27, 2023

Oh, the version should be 0.18.0-dev instead of 0.17.1-dev, right?

Well, for now we have no semver-incompatible changes, so it would be 0.17.1

Copy link
Member

@jedel1043 jedel1043 left a comment

Choose a reason for hiding this comment

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

Thanks!

@nekevss
Copy link
Member

nekevss commented Jul 27, 2023

Well, for now we have no semver-incompatible changes, so it would be 0.17.1

The gc changes that were merged last night had API changes I think, which is why CI is currently failing with 0.17.1-dev as the version.

@Razican Razican added this pull request to the merge queue Oct 22, 2023
@jedel1043 jedel1043 removed this pull request from the merge queue due to a manual request Oct 22, 2023
@jedel1043
Copy link
Member

Saw that the semver action was failing. We should probably fix that before merging

@Razican Razican added the blocked Waiting for another code change label Oct 24, 2023
@Razican
Copy link
Member Author

Razican commented Oct 24, 2023

I have opened obi1kenobi/cargo-semver-checks#570 to track the issue.

@jedel1043
Copy link
Member

@Razican Do you have any updates on this or the linked issue? Maybe this just needs a rebase.

@Razican
Copy link
Member Author

Razican commented Nov 29, 2023

@Razican Do you have any updates on this or the linked issue? Maybe this just needs a rebase.

I didn't have a chance to work on this :( I would like to first test that the PR checks work fine, and that they correctly read the new JSON format.

Then, a rebase would make the trick :)

If you have some time to work on it, please, do it. If not, I'll try to work on it this week or the next.

@obi1kenobi
Copy link

👋 Hi! I'm the maintainer of cargo-semver-checks and I noticed this issue.

As far as I can tell from the logs, the proximate cause of the error is a cargo doc failure to generate rustdoc JSON for boa_icu_provider. The command it ran (and which errored out due to a compilation error) was:

RUSTDOCFLAGS="-Z unstable-options --document-private-items --document-hidden-items --output-format=json --cap-lints allow" cargo doc --package boa_icu_provider --no-deps

It seems plausible based on the log that the cause of the compilation failure was a semver violation in icu_provider or icu_decimal or another crate. If so, a rebase plus cargo update might resolve it, and are worth a shot.

Note that cargo-semver-checks ignores the Cargo.lock file and uses newer dependencies when available and allowed under their version bounds (explanation why). To replicate the failure, it might be necessary to cargo update first.

I would like to first test that the PR checks work fine, and that they correctly read the new JSON format.

I can hopefully save you a step or two here: cargo-semver-checks supports all JSON formats from Rust 1.71 to the most recent nightly. I get emails when a new rustdoc JSON format is merged, and we also test this in CI, so I'm quite sure the issue is not a JSON format incompatibility.

Thanks for checking out cargo-semver-checks! I'm the only maintainer, and it's a donation-supported project, so I'm doing my best to support the community while also providing for my family. Hope you find it useful, and if so, I'd love to know — it would make my day! 😃

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocked Waiting for another code change test Issues and PRs related to the tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants