You are viewing a plain text version of this content. The canonical link for it is here.

Posted to github@datafusion.apache.org by "kosiew (via GitHub)" <gi...@apache.org> on 2026/05/15 04:03:05 UTC

[I] Semver CI should compare against last released API, not main [datafusion]

kosiew opened a new issue, #22192:
URL: https://github.com/apache/datafusion/issues/22192

## Summary
The current breaking-change detector compares pull requests against the base branch (typically main), not against the latest released API surface. This creates semver failures for changes that only modify unreleased APIs and do not break any public, shipped contract.

This issue proposes changing the blocking semver baseline to released versions, with an optional non-blocking advisory comparison to main.

## Current behavior
The semver check is wired to compare against the PR base branch:

1. Workflow runs on pull requests targeting main:
- [.github/workflows/breaking_changes_detector.yml](.github/workflows/breaking_changes_detector.yml#L43)
2. Changed crates are computed against apache/${BASE_REF}:
- [.github/workflows/breaking_changes_detector.yml](.github/workflows/breaking_changes_detector.yml#L73)
3. Semver check is invoked with apache/${BASE_REF}:
- [.github/workflows/breaking_changes_detector.yml](.github/workflows/breaking_changes_detector.yml#L96)
4. Script passes that value directly to cargo semver-checks via --baseline-rev:
- [ci/scripts/changed_crates.sh](ci/scripts/changed_crates.sh#L63)

The comment workflow also states the comparison is against base branch:
- [.github/workflows/breaking_changes_detector_comment.yml](.github/workflows/breaking_changes_detector_comment.yml#L121)

## Problem
Comparing against main can flag API breaks that are only breaks relative to unreleased commits on main, not breaks relative to the latest release users depend on.

This lowers signal quality for a blocking check and can delay valid PRs that simply refine unreleased APIs.

## Concrete example
In #22106, semver flagged removed members on HigherOrderSignature.

Those members were introduced after release 53.1.0 and later changed before release. So the PR is incompatible with current main state, but not incompatible with the last public release API.

## Why this matters
1. Blocking semver should model user-facing released compatibility, not in-progress branch churn.
2. False-positive blocking results increase maintainer and reviewer overhead.
3. Contributors get conflicting signals: bot says break while release contract is still intact.
4. Trust in semver automation drops when failures are not release-relevant.

## Proposed behavior
Use released baselines for blocking semver checks

--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org

Re: [I] Semver CI should compare against last released API, not main [datafusion]

Posted by "gstvg (via GitHub)" <gi...@apache.org>.

gstvg commented on issue #22192:
URL: https://github.com/apache/datafusion/issues/22192#issuecomment-4480869790

   I agree with @rluvaton point about labeling every PR as breaking after one breakage has been merged. On the other hand, the idea of adding the output of the check against the latest release branch in the sticky comment to help identify unreleased items is nice.
   I also opened http://github.com/obi1kenobi/cargo-semver-checks/pull/1639 in upstream so maybe in the future we can have precise reports


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org

Re: [I] Semver CI should compare against last released API, not main [datafusion]

Posted by "rluvaton (via GitHub)" <gi...@apache.org>.

rluvaton commented on issue #22192:
URL: https://github.com/apache/datafusion/issues/22192#issuecomment-4477763951

   If it will add the label for every pr made to crate that had breaking change in main already then the label becomes meaningless and developers will ignore it + it will add confusion 
   
   I think we should the current behavior since it may report breaking changes on something that is already broken but the times this happen will be much less than the alternative
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org

Re: [I] Semver CI should compare against last released API, not main [datafusion]

Posted by "rluvaton (via GitHub)" <gi...@apache.org>.

rluvaton commented on issue #22192:
URL: https://github.com/apache/datafusion/issues/22192#issuecomment-4476851119

   sorry, I'm having hard time to understand.
   
   could you please answer the following questions for each of the cases I list below:
   1. what label will be added/removed if any
   2. what comment will be added/updated/removed if any with what breaking changes being flagged
   3. will it show failure in CI and which ci job?
   
   
   ## Scenario
   Latest released crate has the following code
   ```rust
   pub fn hello() -> u32 {
      1
   }
   
   pub fn world() -> u32 {
      2
   }
   
   pub fn how() -> u32 {
      3
   }
   ```
   
   main is:
   ```rust
   pub fn hello(arg: u32) -> u32 { // <-- added arg - breaking change
      1
   }
   
   pub fn world(a: u32, b: u32) -> u32 { // <-- added 2 args, breaking change
      2
   }
   
   pub fn how() -> u32 {
      3
   }
   ```
   
   which in diff against the released version is:
   ```diff
   -pub fn hello() -> u32 {
   +pub fn hello(arg: u32) -> u32 {
      1
   }
   
   -pub fn world() -> u32 {
   +pub fn world(a: u32, b: u32) -> u32 {
      2
   }
   ```
   
   ## PRs against main
   
   
   <details>
   <summary>Revert all the breaking changes made in main</summary>
   
   #### Diff against main
   ```diff
   -pub fn hello(arg: u32) -> u32 {
   +pub fn hello() -> u32 {
      1
   }
   
   -pub fn world(a: u32, b: u32) -> u32 {
   +pub fn world() -> u32 {
      2
   }
   ```
   
   #### Diff against the released version
   no diff
   
   </details>
   
   
   <details>
   <summary>Revert one of the breaking changes</summary>
   
   #### Diff against main
   ```diff
   -pub fn hello(arg: u32) -> u32 {
   +pub fn hello() -> u32 {
      1
   }
   ```
   
   #### Diff against the released version
   only include the non-reverted breaking change:
   ```diff
   -pub fn world() -> u32 {
   +pub fn world(a: u32, b: u32) -> u32 {
      2
   }
   ```
   
   </details>
   
   <details>
   <summary>add another breaking change that DOES NOT depend on the already breaking changes</summary>
   
   #### Diff against main
   ```diff
   -pub fn how() -> u32 {
   +pub fn how(another: u32) -> u32 {
      3
   }
   ```
   
   #### Diff against the released version
   only existing breaking changes and the new one (adding arg to `how` function)
   ```diff
   -pub fn hello() -> u32 {
   +pub fn hello(arg: u32) -> u32 {
      1
   }
   
   -pub fn world() -> u32 {
   +pub fn world(a: u32, b: u32) -> u32 {
      2
   }
   
   -pub fn how() -> u32 {
   +pub fn how(another: u32) -> u32 {
      3
   }
   ```
   
   </details>
   
   <details>
   <summary>Add non-breaking changes that depend on a breaking change that already exists on main</summary>
   
   #### Diff against main
   ```diff
   pub fn hello(arg: u32) -> u32 {
   +  println!("hello {arg}");
      1
   }
   ```
   
   #### Diff against the released version
   ```diff
   -pub fn hello() -> u32 {
   +pub fn hello(arg: u32) -> u32 {
   +  println!("hello {arg}");
      1
   }
   
   -pub fn world() -> u32 {
   +pub fn world(a: u32, b: u32) -> u32 {
      2
   }
   ```
   
   </details>
   
   <details>
   <summary>Add non-breaking changes to a function that was not broke</summary>
   
   #### Diff against main
   ```diff
   pub fn how() -> u32 {
   +  println!("how");
      3
   }
   ```
   
   #### Diff against the released version
   ```diff
   -pub fn hello() -> u32 {
   +pub fn hello(arg: u32) -> u32 {
      1
   }
   
   -pub fn world() -> u32 {
   +pub fn world(a: u32, b: u32) -> u32 {
      2
   }
   
   pub fn how() -> u32 {
   +  println!("how");
      3
   }
   ```
   
   </details>
   
   <details>
   <summary>Modify existing breaking changes</summary>
   
   #### Diff against main
   ```diff
   -pub fn hello(arg: u32) -> u32 {
   +pub fn hello(arg: usize) -> u32 {
      1
   }
   ```
   
   #### Diff against the released version
   ```diff
   -pub fn hello() -> u32 {
   +pub fn hello(arg: usize) -> u32 {
      1
   }
   
   -pub fn world() -> u32 {
   +pub fn world(a: u32, b: u32) -> u32 {
      2
   }
   
   pub fn how() -> u32 {
      3
   }
   ```
   
   </details>


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org

Re: [I] Semver CI should compare against last released API, not main [datafusion]

Posted by "rluvaton (via GitHub)" <gi...@apache.org>.

rluvaton commented on issue #22192:
URL: https://github.com/apache/datafusion/issues/22192#issuecomment-4457072401

   The reason for base branch is due to not have all breaking changes but only actual ones
   
   And also, think about back porting PRs


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org