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

Make PR comments more customizable #28

Closed
efaulhaber opened this issue Oct 16, 2020 · 6 comments
Closed

Make PR comments more customizable #28

efaulhaber opened this issue Oct 16, 2020 · 6 comments
Labels
discussion Discussing possible future enhancements

Comments

@efaulhaber
Copy link
Contributor

efaulhaber commented Oct 16, 2020

I really like the feature to have the test results in PR comments. However I think these comments can get out of hand on very active PRs.

I would really like to see an option to make the bot only comment once and edit the comment for every new test run like the codecov bot does. This way active PRs wouldn't be full of bot comments.

PR comments should probably also be optional and one should be able to turn them off in the GitHub workflow. I couldn't find anything like this in the readme.

@efaulhaber efaulhaber changed the title Add make PR comments more customizable Make PR comments more customizable Oct 16, 2020
@EnricoMi
Copy link
Owner

I agree on these features. Reusing the comment on a very active PR means that you will find test results at the very top of the PR page, or is the comment moving down when edited by codecov?

@efaulhaber
Copy link
Contributor Author

efaulhaber commented Oct 16, 2020

Yes, unfortunately the comment is staying on the very top in codecov. I still like this better than having dozens of bot comments across the PR.

Maybe automatically deleting the old comment could be an option? But I guess this would show delete messages like the one below and it would probably notify everyone who subscribed to the PR. If that were possible, codecov would have probably implemented it.

Edit: Maybe the best way would be to add these options:

  1. Completely disable PR comments.
  2. Only comment once and edit the comment.
  3. Comment if the bot's comment is not at the bottom, but edit the bottom comment otherwise.

Choosing last option would make the comment always stay at the bottom but disable double posts by the bot and make it only comment again if someone else commented the PR since the last test.
I would still choose the second option, but people may want to have the test results at the bottom.

@EnricoMi
Copy link
Owner

I agree with option 1. and 2. What about this alternative to option 3. as the default: Currently, when a branch gets rebased and commits are no longer part of the branch, then the bot hides all test results for those commits. For an example, see horovod/horovod#2373. This behaviour could be extended to always hide earlier comments.

On your 3. option: Since every commit will make the latest result comment move upward, it might eventually move out of sight. And an active PR involves review comments. So I see the third option a niche where it will almost always comment anyway. I think hiding is the best option to make it less polluted. What do you think?

Deleting is not an option as you pointed out, as it does leave a hint about the deleted comment. So this does not clean up the PR.

@efaulhaber
Copy link
Contributor Author

efaulhaber commented Oct 16, 2020

Are you sure that there is no way for the bot to "secretly" delete a comment? If so, I like the idea of hiding older comments by default.

For the third option I would ignore commits at all as they don't move the comment far up. Even after 10 commits without any comments the test report would still be easily visible (and I would say that 10 commits without any comments are an exception). I was thinking about the bot commenting again when the test report was moved up by other comments and then someone pushes new commits.

For example in one of my repositories it looks like this:
grafik

I pushed several commits before I requestet a review to start a discussion. My idea is that in this case there would be one comment and the three commits below, so no new comment for every new commit.

Once someone comments I would make the bot comment again (after a test is run) because review comments can move the bot comment down a lot.

@EnricoMi
Copy link
Owner

I agree, third option to stay the latest comment makes sense. I will create three separate issue to track them individually. Thanks for the ideas and discussion, this is a really great improvement.

@EnricoMi
Copy link
Owner

@erik-f I have raised a few issues for the different comment behaviours (see linked issues above). I have also created a few more: #40 and #41.

Please follow issues if interested or feel free to contribute. Thanks for your great input!

@EnricoMi EnricoMi added discussion Discussing possible future enhancements and removed enhancement New feature or request labels Oct 25, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion Discussing possible future enhancements
Projects
None yet
Development

No branches or pull requests

2 participants