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

Gradle check single comments for autobot #8488

Closed

Conversation

stephen-crawford
Copy link
Contributor

@stephen-crawford stephen-crawford commented Jul 6, 2023

Description

This change updates the GitHub Actions comment system. The existing system makes individual comments each run which makes the conversation chains of PRs long. This addresses this issue.

Related Issues

opensearch-project/.github#171

Check List

  • New functionality includes testing.
    • All tests pass
  • New functionality has been documented.
    • New functionality has javadoc added
  • Commits are signed per the DCO using --signoff
  • Commit changes are listed out in CHANGELOG.md file (See: Changelog)

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

Signed-off-by: Stephen Crawford <[email protected]>
@reta
Copy link
Collaborator

reta commented Jul 6, 2023

@scrawfor99 the comments provide useful information regarding the Jenkins builds so I could quickly check the failed ones, this change overrides the build status comment with the latest all the time, is that right? Essentially, the references for older builds are lost ...

Signed-off-by: Stephen Crawford <[email protected]>
@stephen-crawford
Copy link
Contributor Author

stephen-crawford commented Jul 6, 2023

Hi @reta, this change should make it so that a subsequent failing build will replace an old build failure comment body with the new information. Currently, a build failure triggers a new comment no matter what. This means that on PRs which are long standing or suffer from flaky tests, you can run into lots of comments effectively saying the same thing. This would swap this behavior to overwrite the text body of an existing comment with the new information. So after a comment is created, all subsequent failures would replace that comment body with the updated status.

You should still be able to access the build information of previous runs from the small "X" marks on the right side of the conversation view next to the commit IDs. I admit this is less convenient than the large comment blocks but it is much easier to parse in my opinion. I also think that for the most part, because the build comments provide no readable information about the build in question (they don't have a name associated with them etc., just a URL), you are likely to find the commit-based references more valuable if you wanted to refer to a specific change you made some time ago.

Let me know what you think. If I am in the minority here for wanting this change, then no problem--I just thought it may be something useful for general development purposes.

I am also not a maintainer so obviously have less say in this matter. If you don't think it is a good idea, no worries.

Signed-off-by: Stephen Crawford <[email protected]>
@github-actions
Copy link
Contributor

github-actions bot commented Jul 6, 2023

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

github-actions bot commented Jul 6, 2023

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

github-actions bot commented Jul 6, 2023

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

github-actions bot commented Jul 6, 2023

Gradle Check (Jenkins) Run Completed with:

  • RESULT: UNSTABLE ❕
  • TEST FAILURES:
      1 org.opensearch.remotestore.SegmentReplicationRemoteStoreIT.testPressureServiceStats
      1 org.opensearch.remotestore.RemoteStoreIT.testStaleCommitDeletionWithInvokeFlush

@reta
Copy link
Collaborator

reta commented Jul 6, 2023

Let me know what you think. If I am in the minority here for wanting this change, then no problem--I just thought it may be something useful for general development purposes.

Thanks @scrawfor99 , it is really useful for me (to have comment per build/check), and also Github sends the notifications when new comment is added (so pull request needs to be looked at), I think with the comment update no notifications will be sent.

But I could certainly live without it, if others think is it noise.

@kotwanikunal
Copy link
Member

kotwanikunal commented Jul 6, 2023

+1 on what @reta said.
I think as a reviewer, it makes it easy for me to glance at the gradle check runs without having to figure out and going through all the checks manually to see if the change impacts other areas (For eg a test which is flaky repeatedly on a single PR).
And usually gradle check notifications do lead me to a PR as well :)

@owaiskazi19
Copy link
Member

Thanks for adding this @scrawfor99.
+1 to @reta and @kotwanikunal . I think it's fine to have multiple build comments on the PR to provide step wise failure/success logs of the commits.

@stephen-crawford
Copy link
Contributor Author

Closing since feature is not desired.

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