Skip to content

Added MSRV check#3291

Merged
nekevss merged 3 commits intomainfrom
test_msrv
Sep 24, 2023
Merged

Added MSRV check#3291
nekevss merged 3 commits intomainfrom
test_msrv

Conversation

@Razican
Copy link
Member

@Razican Razican commented Sep 20, 2023

This Pull Request is a follow-up of #3290.

It adds a test for the MSRV to the CI, so that we don't get similar issues in the future.

@Razican Razican added the C-Tests Issues and PRs related to the tests. label Sep 20, 2023
@Razican Razican added this to the v0.18.0 milestone Sep 20, 2023
@Razican Razican requested a review from a team September 20, 2023 19:40
@jedel1043
Copy link
Member

jedel1043 commented Sep 20, 2023

Can we read the root Cargo.toml from CI? it would be ideal to check using the provided rust-version from our config.

@github-actions
Copy link

Test262 conformance changes

Test result main count PR count difference
Total 95,574 95,574 0
Passed 75,084 75,084 0
Ignored 19,494 19,494 0
Failed 996 996 0
Panics 0 0 0
Conformance 78.56% 78.56% 0.00%

@codecov
Copy link

codecov bot commented Sep 20, 2023

Codecov Report

Patch coverage has no change and project coverage change: -0.11% ⚠️

Comparison is base (986b048) 50.32% compared to head (37e0547) 50.21%.
Report is 4 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3291      +/-   ##
==========================================
- Coverage   50.32%   50.21%   -0.11%     
==========================================
  Files         436      436              
  Lines       42604    42691      +87     
==========================================
  Hits        21439    21439              
- Misses      21165    21252      +87     

see 6 files with indirect coverage changes

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

@nekevss
Copy link
Member

nekevss commented Sep 21, 2023

Second the idea regarding reading the version in the Cargo.toml if possible.

@Razican
Copy link
Member Author

Razican commented Sep 22, 2023

Will see how to do this this weekend :)

@Razican
Copy link
Member Author

Razican commented Sep 23, 2023

@jedel1043, @nekevss this should be ready now :)

Copy link
Member

@nekevss nekevss left a comment

Choose a reason for hiding this comment

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

Was double checking the run and noticed a deprecation warning. Maybe adjust it to use $GITHUB_OUTPUT?

Co-authored-by: Kevin <46825870+nekevss@users.noreply.github.com>
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.

Nice improvement!

Copy link
Member

@HalidOdat HalidOdat left a comment

Choose a reason for hiding this comment

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

Looks perfect to me! :)

Copy link
Member

@nekevss nekevss left a comment

Choose a reason for hiding this comment

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

LGTM

@nekevss nekevss added this pull request to the merge queue Sep 23, 2023
Merged via the queue into main with commit 7e18da6 Sep 24, 2023
@jedel1043 jedel1043 deleted the test_msrv branch September 24, 2023 00:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

C-Tests Issues and PRs related to the tests.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants