Skip to content

Conversation

gruebel
Copy link
Member

@gruebel gruebel commented May 16, 2025

Changes

Merge requirement checklist

  • CONTRIBUTING guidelines followed
  • Unit tests added/updated (if applicable)
  • Appropriate CHANGELOG.md files updated for non-trivial, user-facing changes
  • Changes in public API reviewed (if applicable)

@gruebel gruebel requested a review from a team as a code owner May 16, 2025 21:00
Copy link

codecov bot commented May 16, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 81.3%. Comparing base (9a0099a) to head (7493403).
Report is 11 commits behind head on main.

Additional details and impacted files
@@          Coverage Diff          @@
##            main   #2976   +/-   ##
=====================================
  Coverage   81.3%   81.3%           
=====================================
  Files        126     126           
  Lines      24375   24375           
=====================================
  Hits       19827   19827           
  Misses      4548    4548           

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Contributor

@hdost hdost left a comment

Choose a reason for hiding this comment

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

Same as comment on other repo

@cijothomas cijothomas requested a review from Copilot May 22, 2025 03:55
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR updates the MSRV check process by removing legacy scripts and leveraging a fallback resolver configuration while updating dependency-related tooling.

  • Removed scripts: patch_dependencies.sh and msrv_config.json
  • Refactored msrv.sh to use cargo metadata for workspace member detection and updated the MSRV verification process
  • Updated CI workflow to use newer toolchains and rely on external actions for dependency checks
  • Introduced a .cargo/config.toml with a fallback resolver for incompatible Rust versions

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
scripts/patch_dependencies.sh Deleted; dependency patching is removed in favor of external tooling.
scripts/msrv_config.json Deleted; the configuration is no longer required with the new approach in msrv.sh.
scripts/msrv.sh Updated to dynamically gather workspace manifests from cargo metadata and verify MSRV via cargo msrv verify.
.github/workflows/ci.yml Updated CI steps to utilize new actions/toolchains and removed direct script invocations.
.cargo/config.toml Added new resolver settings to support fallback for incompatible Rust versions.
Comments suppressed due to low confidence (4)

scripts/patch_dependencies.sh:1

  • [nitpick] Confirm that the removal of patch_dependencies.sh is intentional and that dependency patching is now handled by the new tooling in the CI workflow.
# Entire file removed

scripts/msrv_config.json:1

  • [nitpick] Ensure that the removal of msrv_config.json aligns with the new workspace-based MSRV verification approach and does not affect any other configuration dependencies.
# Entire file removed

.github/workflows/ci.yml:101

  • [nitpick] Verify that updating the nightly toolchain to 2025-05-04 is compatible with all CI tasks and aligns with the project's requirements.
toolchain: nightly-2025-05-04

.github/workflows/ci.yml:103

  • [nitpick] Ensure that switching to using the taiki-e/install-action for cargo-check-external-types fully covers the functionality previously provided by patch_dependencies.sh.
- uses: taiki-e/install-action@33734a118689b0b418824fb78ea2bf18e970b43b # v2.50.4

#!/bin/bash

set -eu

Copy link

Copilot AI May 22, 2025

Choose a reason for hiding this comment

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

[nitpick] Consider adding a comment to document that the script now retrieves all workspace member manifests using cargo metadata, clarifying the new MSRV verification scope.

Suggested change
# Retrieve all workspace member manifests using `cargo metadata`.
# This defines the scope of MSRV (Minimum Supported Rust Version) verification.

Copilot uses AI. Check for mistakes.

@@ -0,0 +1,3 @@
[resolver]
Copy link

Copilot AI May 22, 2025

Choose a reason for hiding this comment

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

[nitpick] Consider expanding the comment in this config to clearly document when and why the 'fallback' resolver is used, for enhanced clarity in future maintenance.

Copilot uses AI. Check for mistakes.

@gruebel
Copy link
Member Author

gruebel commented May 22, 2025

Not sure why I was able to push directly to the main repo, but I'm not able to update the branch anymore 😅 Closing in favor of #2993

@gruebel gruebel closed this May 22, 2025
lucamuscat added a commit to lucamuscat/opentelemetry-rust that referenced this pull request Sep 15, 2025
With a recent breaking update in serde (PR open-telemetry#2976), the `Deserialize`
derive attribute now requires `std` support in order to compile due to
some internal logic inside of serde now being moved behind a feature
gate.
lucamuscat added a commit to lucamuscat/opentelemetry-rust that referenced this pull request Sep 15, 2025
With a recent breaking update in serde (PR open-telemetry#2976), the `Deserialize`
derive attribute now requires `std` support in order to compile due to
some internal logic inside of serde now being moved behind a feature
gate.
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