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

feat!: remove SUCCESS comments from PRs/MRs #526

Merged
merged 1 commit into from
Jan 14, 2025
Merged

feat!: remove SUCCESS comments from PRs/MRs #526

merged 1 commit into from
Jan 14, 2025

Conversation

maxrake
Copy link
Contributor

@maxrake maxrake commented Jan 13, 2025

This change removes the SUCCESS comments from pull requests (PRs) and merge requests (MRs). Other comment types are unchanged. This change is necessary because the SUCCESS comments are considered by existing customers to be too noisy and not useful since the related status check will show as passing when the analyis is successful. Plus, the logs contain the same information as was included in the SUCCESS comments.

Closes #78

BREAKING CHANGE: PRs/MRs will no longer have SUCCESS comments.

Checklist

  • Does this PR have an associated issue (i.e., closes #<issueNum> in description above)?
  • Have you ensured that you have met the expected acceptance criteria?
  • Have you created sufficient tests?
  • Have you updated all affected documentation?

Testing

The changes in this PR are available for testing with the maxrake/phylum-ci:quiet_wins Docker image found on Docker Hub.


This is what it looks like when a PR is created on GitHub with a lockfile that includes changes where the analysis is successful. Notice there is no comment.

image

This is what it looks like when the PR is updated to include a known bad dependency:

image

This is what it looks like when the PR is updated to remove the known bad dependency. Notice that the status check is green and there is no comment.

image

TODO

This change removes the `SUCCESS` comments from pull requests (PRs) and
merge requests (MRs). Other comment types are unchanged. This change is
necessary because the `SUCCESS` comments are considered by existing
customers to be too noisy and not useful since the related status check
will show as passing when the analyis is successful. Plus, the logs
contain the same information as was included in the `SUCCESS` comments.

Closes #78

BREAKING CHANGE: PRs/MRs will no longer have `SUCCESS` comments.
@maxrake maxrake requested a review from a team as a code owner January 13, 2025 21:34
@maxrake maxrake requested a review from mhorner-vera January 13, 2025 21:34
maxrake added a commit to phylum-dev/phylum-analyze-pr-action that referenced this pull request Jan 13, 2025
This change updates the documentation for the GitHub Action to match the
wording provided in phylum-dev/phylum-ci#526.
Copy link
Contributor

@louislang louislang left a comment

Choose a reason for hiding this comment

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

Short and sweet 👍

@maxrake maxrake merged commit 6cca5bd into main Jan 14, 2025
13 checks passed
@maxrake maxrake deleted the quiet_wins branch January 14, 2025 16:18
maxrake added a commit to phylum-dev/phylum-analyze-pr-action that referenced this pull request Jan 14, 2025
This change updates the documentation for the GitHub Action to match the
wording provided in phylum-dev/phylum-ci#526.
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.

Remove SUCCESS comments for phylum-ci
3 participants