-
Notifications
You must be signed in to change notification settings - Fork 947
Mixed version compatibility detection #6477
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
Merged
Merged
Changes from all commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
7de151f
Add mixed version compatibility detection workflow
RanVaknin b4cf906
Improve mixed version detection: filter false positives
RanVaknin 47d0582
Fix false positive filtering for block comments
RanVaknin 74eeb2a
Address feedback: show only changed class names in error message
RanVaknin 915f93b
Add comments, expand exclusion of patterns
RanVaknin File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
104 changes: 104 additions & 0 deletions
104
.github/workflows/mixed-version-compatibility-review.yml
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,104 @@ | ||
| name: Mixed Version Compatibility Review | ||
|
|
||
| permissions: | ||
| contents: read | ||
| pull-requests: read | ||
|
|
||
| on: | ||
| merge_group: | ||
| pull_request: | ||
| types: [ opened, synchronize, reopened, labeled, unlabeled ] | ||
| branches: | ||
| - master | ||
|
|
||
| jobs: | ||
| mixed-version-compatibility-check: | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - uses: actions/checkout@v4 | ||
| with: | ||
| fetch-depth: 0 | ||
|
|
||
| - name: Check for mixed version compatibility risks | ||
| id: compatibility-check | ||
| run: | | ||
| git fetch origin ${{ github.base_ref }} --depth 1 | ||
|
|
||
| # Define the specific base class files that are risky for mixed versions | ||
| BASE_CLASS_FILES="core/aws-core/src/main/java/software/amazon/awssdk/awscore/AwsRequest.java | ||
| core/aws-core/src/main/java/software/amazon/awssdk/awscore/AwsResponse.java | ||
| core/aws-core/src/main/java/software/amazon/awssdk/awscore/AwsResponseMetadata.java | ||
| core/aws-core/src/main/java/software/amazon/awssdk/awscore/exception/AwsServiceException.java | ||
| core/sdk-core/src/main/java/software/amazon/awssdk/core/SdkPojo.java" | ||
|
|
||
| # Check if any of the base class files were modified | ||
| CHANGED_BASE_FILES=$(git diff --name-only remotes/origin/${{ github.base_ref }} -- $BASE_CLASS_FILES || true) | ||
|
|
||
| if [ -z "$CHANGED_BASE_FILES" ]; then | ||
| echo "No base class changes detected." | ||
| echo "has_risk=false" >> $GITHUB_OUTPUT | ||
| exit 0 | ||
| fi | ||
|
|
||
| echo "Base class changes detected in:" | ||
| echo "$CHANGED_BASE_FILES" | ||
|
|
||
| # Look for new public methods in the changed base class files | ||
| # Filter out obvious false positives: comments, string literals, javadoc | ||
| NEW_METHODS=$(git diff remotes/origin/${{ github.base_ref }} -- $BASE_CLASS_FILES | \ | ||
|
|
||
| grep '^+.*public.*(' | \ # Find lines with new public methods | ||
| grep -v '^+[[:space:]]*//.*' | \ # Line comments | ||
| grep -v '^+[[:space:]]*\*.*' | \ # Javadoc lines | ||
| grep -v '^+[[:space:]]*/\*.*' || true # Block comments | ||
|
|
||
| if [ -n "$NEW_METHODS" ]; then | ||
| echo "::warning::New public methods detected in base classes:" | ||
| echo "$NEW_METHODS" | while read line; do | ||
| echo "::warning::$line" | ||
| done | ||
| echo "has_risk=true" >> $GITHUB_OUTPUT | ||
| echo "risk_type=new_methods" >> $GITHUB_OUTPUT | ||
| else | ||
| echo "::warning::Base class files modified but no new public methods detected" | ||
| echo "has_risk=true" >> $GITHUB_OUTPUT | ||
| echo "risk_type=other_changes" >> $GITHUB_OUTPUT | ||
| fi | ||
|
|
||
| - name: Fail if compatibility risks found without approval | ||
| if: ${{ steps.compatibility-check.outputs.has_risk == 'true' && !contains(github.event.pull_request.labels.*.name, 'mixed-version-compatibility-reviewed') }} | ||
| run: | | ||
| # Define the base class files | ||
| BASE_CLASS_FILES="core/aws-core/src/main/java/software/amazon/awssdk/awscore/AwsRequest.java | ||
| core/aws-core/src/main/java/software/amazon/awssdk/awscore/AwsResponse.java | ||
| core/aws-core/src/main/java/software/amazon/awssdk/awscore/AwsResponseMetadata.java | ||
| core/aws-core/src/main/java/software/amazon/awssdk/awscore/exception/AwsServiceException.java | ||
| core/sdk-core/src/main/java/software/amazon/awssdk/core/SdkPojo.java" | ||
|
|
||
| # Extract class names from the actually changed files | ||
| CHANGED_BASE_FILES=$(git diff --name-only remotes/origin/${{ github.base_ref }} -- $BASE_CLASS_FILES || true) | ||
| CHANGED_CLASS_NAMES=$(echo "$CHANGED_BASE_FILES" | sed 's|.*/||' | sed 's|\.java||' | paste -sd ',' - | sed 's/,/, /g') | ||
|
|
||
| echo "::error::Mixed version compatibility risk detected!" | ||
| echo "::error::Changes were made to base classes that generated service code implements:" | ||
| echo "::error::- $CHANGED_CLASS_NAMES" | ||
| echo "::error::" | ||
| echo "::error::This may break customers using mixed SDK versions if:" | ||
| echo "::error::- New methods are added with UnsupportedOperationException defaults" | ||
| echo "::error::- Core behavior changes invoke existing methods in new ways" | ||
| echo "::error::- Interface contracts change in subtle ways" | ||
| echo "::error::" | ||
| echo "::error::Please review with the team for mixed version impact and add" | ||
| echo "::error::'mixed-version-compatibility-reviewed' label after approval." | ||
| echo "::error::" | ||
| echo "::error::If this introduces compatibility issues, consider:" | ||
| echo "::error::- Bumping minor version" | ||
| echo "::error::- Documenting compatibility impact in release notes" | ||
| echo "::error::- Ensuring older service modules can handle the changes" | ||
| exit 1 | ||
|
|
||
| - name: Success message when approved | ||
| if: ${{ steps.compatibility-check.outputs.has_risk == 'true' && contains(github.event.pull_request.labels.*.name, 'mixed-version-compatibility-reviewed') }} | ||
| run: | | ||
| echo "✅ Mixed version compatibility risk detected but approved for merge" | ||
| echo "Base class changes have been reviewed and approved by the team" | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.