runatlantis / atlantis

Terraform Pull Request Automation
https://www.runatlantis.io
Other
7.87k stars 1.06k forks source link

Support posting PR comments in reverse order for VCS #2547

Open wpbeckwith opened 2 years ago

wpbeckwith commented 2 years ago

Community Note


Describe the user story

When Atlantis posts the terraform output back to a PR, if the output exceeds the single comment limit, then atlantis will post several comments. However the order of these comments is sub-optimal. Assuming atlantis needs to post 3 comments for the full output then the PR will read like

5 # start of 3rd comment
...
6 # end of 3rd comment
3 # start of 2nd comment
...
4 # end of 2nd comment
1 # start of 1st comment
...
2 # end of 1st comment

The current ordering requires a user to scan to section 1 and read to section 2, then scroll back up past sections 1 and 4 to find section 3 and read to 4, then repeat again and scroll to section 5 and finally finish at 6.

Describe the solution you'd like

Atlantis should reverse the order of the array before it posts the comments, then the PR will read like

1 # start of 1st comment
...
2 # end of 1st comment
3 # start of 2nd comment
...
4 # end of 2nd comment
5 # start of 3rd comment
...
6 # end of 3rd comment

This now reads more naturally from top to bottom. Even if another PR comment somehow was included while Atlantis was posting its comments, it would still retain the easier top down reading.

Describe the drawbacks of your solution

This section is important not only to identify future issues, but also for us to see whether you thought through your request. When filling it, ask yourself what are the problems we could have maintaining what you propose. How often will it break?

None known as this should be an option to reverse the posting behavior.

Describe alternatives you've considered

We have implemented the change here, https://github.com/runatlantis/atlantis/commit/66a21de4946208f167df5f926f7949a43947eb3d, minus a server config option to control the reversal.

jamengual commented 2 years ago

this only happens in bitbucket? I think I have seen this on github long ago, and just last week I was testing the max post size and it seemed that github can do far more that it is coded which could reduce the additional comments, but it will need testing.

On Sun, Oct 2, 2022, 9:36 a.m. wpbeckwith @.***> wrote:

Community Note

  • Please vote on this issue by adding a 👍 reaction https://blog.github.com/2016-03-10-add-reactions-to-pull-requests-issues-and-comments/ to the original issue to help the community and maintainers prioritize this request. Searching for pre-existing feature requests helps us consolidate datapoints for identical requirements into a single place, thank you!
  • Please do not leave "+1" or other comments that do not add relevant new information or questions, they generate extra noise for issue followers and do not help prioritize the request.
  • If you are interested in working on this issue or have submitted a pull request, please leave a comment.

Describe the user story

When Atlantis posts the terraform output back to a PR, if the output exceeds the single comment limit, then atlantis will post several comments. However the order of these comments is sub-optimal. Assuming atlantis needs to post 3 comments for the full output then the PR will read like

5 # start of 3rd comment

...

6 # end of 3rd comment

3 # start of 2nd comment

...

4 # end of 2nd comment

1 # start of 1st comment

...

2 # end of 1st comment

The current ordering requires a user to scan to section 1 and read to section 2, then scroll back up past sections 1 and 4 to find section 3 and read to 4, then repeat again and scroll to section 5 and finally finish at 6.

Describe the solution you'd like

Atlantis should reverse the order of the array before it posts the comments, then the PR will read like

1 # start of 1st comment

...

2 # end of 1st comment

3 # start of 2nd comment

...

4 # end of 2nd comment

5 # start of 3rd comment

...

6 # end of 3rd comment

This now reads more naturally from top to bottom. Even if another PR comment somehow was included while Atlantis was posting its comments, it would still retain the easier top down reading.

Describe the drawbacks of your solution

This section is important not only to identify future issues, but also for us to see whether you thought through your request. When filling it, ask yourself what are the problems we could have maintaining what you propose. How often will it break?

None known as this should be an option to reverse the posting behavior.

Describe alternatives you've considered

We have implemented the change here, 66a21de https://github.com/runatlantis/atlantis/commit/66a21de4946208f167df5f926f7949a43947eb3d, minus a server config option to control the reversal.

— Reply to this email directly, view it on GitHub https://github.com/runatlantis/atlantis/issues/2547, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAQ3ERH2AFSFHEYDSIYXWQLWBG22VANCNFSM6AAAAAAQ27BNOE . You are receiving this because you are subscribed to this thread.Message ID: @.***>

wpbeckwith commented 2 years ago

I haven't tested this with GH, only BitBucket because that is what we use FTM. However, next year we are looking to migrate to GHE.

wpbeckwith commented 2 years ago

Honestly if Atlantis currently post for all the servers like this, then wrapping this in a config option should be a big win for all the VCS. As for testing, the code simply reverses the current slice order. If you think this is good, then I can implement the config option and some unit tests for this?

jamengual commented 2 years ago

that should work.

On Sun, Oct 2, 2022, 11:41 a.m. wpbeckwith @.***> wrote:

Honestly if Atlantis currently post for all the servers like this, then wrapping this in a config option should be a big win for all the VCS. As for testing, the code simply reverses the current slice order. If you think this is good, then I can implement the config option and some unit tests for this?

— Reply to this email directly, view it on GitHub https://github.com/runatlantis/atlantis/issues/2547#issuecomment-1264707451, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAQ3ERBK7754QGEJIZD3FJLWBHJMXANCNFSM6AAAAAAQ27BNOE . You are receiving this because you commented.Message ID: @.***>

wpbeckwith commented 2 years ago

Ok, I've started working on this and I have written the reverse function and test in the server/events/vcs/common package and created the server configuration flag. What I'm stuck on is how to read the server configuration flag at the point where common.SplitComment(...) is called, like line 139 in the server/events/vcs/bitbucketserver/client.go file.

jamengual commented 2 years ago

@nitrocode do you have an idea for this?

nitrocode commented 2 years ago

@wpbeckwith could you submit a draft PR for now and we can try to comment on it so you can fine tune it?

wpbeckwith commented 1 year ago

I updated the title since this really is an option for any VCS that splits PR comments due to the size of the comment.