-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Support posting PR comments in reverse order for VCS #2547
Comments
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.
------------------------------
- I'd be willing to implement this feature (contributing guide
<https://github.com/runatlantis/atlantis/blob/master/CONTRIBUTING.md>)
*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
<66a21de>,
minus a server config option to control the reversal.
—
Reply to this email directly, view it on GitHub
<#2547>, or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAQ3ERH2AFSFHEYDSIYXWQLWBG22VANCNFSM6AAAAAAQ27BNOE>
.
You are receiving this because you are subscribed to this thread.Message
ID: ***@***.***>
|
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. |
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? |
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
<#2547 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAQ3ERBK7754QGEJIZD3FJLWBHJMXANCNFSM6AAAAAAQ27BNOE>
.
You are receiving this because you commented.Message ID:
***@***.***>
|
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 |
@nitrocode do you have an idea for this? |
@wpbeckwith could you submit a draft PR for now and we can try to comment on it so you can fine tune it? |
I updated the title since this really is an option for any VCS that splits PR comments due to the size of the comment. |
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
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
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, minus a server config option to control the reversal.
The text was updated successfully, but these errors were encountered: