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

PRs tagged as breaking with ! should have minor changes in changeset #2867

Closed
nedsalk opened this issue Jul 30, 2024 · 2 comments · Fixed by #2987
Closed

PRs tagged as breaking with ! should have minor changes in changeset #2867

nedsalk opened this issue Jul 30, 2024 · 2 comments · Fixed by #2987
Assignees
Labels
bug Issue is a bug

Comments

@nedsalk
Copy link
Contributor

nedsalk commented Jul 30, 2024

#2863 has a breaking changes section due to #2777 but it's not a minor bump. This should have been caught in the CI. We have logic to verify that, if the changeset file has minor changes that the PR should have the ! suffix, but we don't have the inverse check that if a PR is tagged as breaking that the changesets should contain minor changes.

@nedsalk nedsalk added the bug Issue is a bug label Jul 30, 2024
@arboleya arboleya added the p1 label Jul 30, 2024
@mvares
Copy link
Contributor

mvares commented Aug 1, 2024

@nedsalk, I assume that this solution could resolve it:

# Check if the PR title contains an exclamation mark (breaking)
if echo "$PR_TITLE" | grep -q '!'; then
  # If the PR is breaking, ensure that the changeset contains minor changes
  if ! echo "$AFFECTED_PACKAGES" | grep -q 'minor'; then
    echo "PR is marked as breaking but changeset does not contain minor changes."
    echo "Changeset file: $CHANGESET_FILE"
    echo "Changeset content:"
    cat "$CHANGESET_FILE"
    exit 1
  fi
else
  # If the PR is not breaking, ensure that minor changes are not present in the changeset
  if echo "$AFFECTED_PACKAGES" | grep -q 'minor'; then
    echo "Changeset has `minor` changes. Please mark the PR as breaking by adding an exclamation mark to its title (e.g. fix!: xxx) or use a `patch` instead."
    exit 1
  fi
fi

Flow:

sequenceDiagram
    participant PR as Pull Request
    participant CI as Continuous Integration
    participant Changeset as Changeset File

    %% PR with title containing '!'
    PR->>CI: PR Title with !
    CI->>Changeset: Check for minor changes
    Changeset->>CI: Contains minor changes?
    CI->>PR: If yes, proceed
    CI->>PR: If no, error (Changeset should indicate at least a minor bump)

    %% PR with title without '!'
    PR->>CI: PR Title without !
    CI->>Changeset: Check for minor changes
    Changeset->>CI: Contains minor changes?
    CI->>PR: If yes, error (PR title should reflect breaking changes)
    CI->>PR: If no, proceed

    %% Minor Changes Check
    Changeset->>CI: Indicates minor bump?
    CI->>PR: If no, error (PR title should have '!')
    CI->>PR: If yes, proceed

Loading

@nedsalk
Copy link
Contributor Author

nedsalk commented Aug 1, 2024

Yes, please create a PR for this if you want.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Issue is a bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants