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: Enable hide-prev-plan-comments Feature for BitBucket Cloud #4495

Open
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

ragne
Copy link

@ragne ragne commented Apr 30, 2024

what

Atlantis produces a truly ungodly amount of comments during an extensive work on a PR in the bitbucket. For GH integration, there's an ability to collapse comments that are no longer relevant for a discussion (i.e. old plans that are now discarded, etc).
This PR tries to add that to bitbucket with all its limitations. Since BB doesn't have an ability to collapse comments, the only way there is to delete old comments all-together.
In my opinion, that is fine, since this feature is gated via a cmdline flag anyway.

Please let me know your thoughts. We used this code internally at $dayjob and it gets job done.

why

It's just annoying to scroll a kilometere-long comment just to find another one right after. The plans that we're working with are more than 2-3k lines. We used tf-summarize to reduce the amount of text in the comment, but the main problem of having too much comments from atlantis still stands.

tests

We have used the code in our own installation. I'll be honest, go isn't my strongest language, so I have no idea if I have to add tests to the codebase and how to do that.

references

@ragne ragne requested review from a team as code owners April 30, 2024 08:56
@ragne ragne requested review from chenrui333, lukemassa and nitrocode and removed request for a team April 30, 2024 08:56
@github-actions github-actions bot added go Pull requests that update Go code provider/bitbucket labels Apr 30, 2024
@ragne ragne changed the title hide-bb-comments: This should allow to delete previous comments in a PR fix: This should allow to delete previous comments in a PR Apr 30, 2024
@jamengual
Copy link
Contributor

@ragne could you please add tests and docs for this?

@jamengual jamengual added the waiting-on-response Waiting for a response from the user label Apr 30, 2024
@jippi
Copy link
Contributor

jippi commented May 3, 2024

I've been considering doing something similar for GitLab—configuring Atlantis to delete previous comments (or retain up to X of them). Is there an opportunity (and desire) to make this work across SCM providers?

Some of our GitLab MRs exceed 4-5 MB of Atlantis output, putting my M1 laptop, Chrome, and Gitlab UI under so much pressure that I worry they will eventually become diamonds.

@jamengual
Copy link
Contributor

jamengual commented May 3, 2024 via email

return pulls.Values, nil
}

func (b *Client) GetMyUUID() (uuid string, err error) {
Copy link
Contributor

@jippi jippi May 3, 2024

Choose a reason for hiding this comment

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

Can we cache this? or only pull it on boot and store as a field value.

Not likely the UUID of the Atlantis user changes?

@X-Guardian
Copy link
Contributor

@jippi, Atlantis already support hiding previous comments in GitLab. https://www.runatlantis.io/docs/server-configuration.html#hide-prev-plan-comments

@X-Guardian X-Guardian changed the title fix: This should allow to delete previous comments in a PR feat: Enable hide-prev-plan-comments Feature for BitBucket Cloud May 4, 2024
@X-Guardian
Copy link
Contributor

I changed the PR title to make the intended change clearer.

@X-Guardian
Copy link
Contributor

Can you add tests for this?

@jippi
Copy link
Contributor

jippi commented May 4, 2024

@X-Guardian GitLab does indeed support hiding them, but they are still included in the initial page load, and thus add significant weight to the page size and performance - sadly its not as cleverly done as GitHub that doesn't load them at all, until you click on them and then JIT load them for you.

In GitLab its just wrapped in a details, making them visually smaller (but still quite large compared to GitHub), so the ability to "hide" via "delete" as proposed in this change, is something I've been keen on getting implemented in GitLab as well :)

Some of our GitLab MR pages are 5-10 MB TF output sent to the browser within those detail blocks, making it quite heavy on both end-user devices and the GitLab service itself.

@github-actions github-actions bot added the docs Documentation label May 7, 2024
@ragne
Copy link
Author

ragne commented May 7, 2024

I'm not sure if the implementation there conforms to the name of the cmdline flag. If you want to create a separate flag (instead of hide-prev-plan-comments), please let me know.
I honestly have no idea how to write tests in go, so I just copied whatever was there :)

@ragne ragne requested a review from jippi May 21, 2024 09:20
@github-actions github-actions bot added build Relating to how we build Atlantis dependencies PRs that update a dependency file provider/github github-actions website labels May 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build Relating to how we build Atlantis dependencies PRs that update a dependency file docs Documentation github-actions go Pull requests that update Go code provider/bitbucket provider/github waiting-on-response Waiting for a response from the user website
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants