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 13 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

@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
Copy link
Author

ragne commented Jun 17, 2024

Hey @chenrui333 @lukemassa @jamengual @X-Guardian is there anything that blocks this still?

Copy link

This issue is stale because it has been open for 1 month with no activity. Remove stale label or comment or this will be closed in 1 month.

@github-actions github-actions bot added the Stale label Jul 19, 2024
@jamengual jamengual removed dependencies PRs that update a dependency file go Pull requests that update Go code provider/github github-actions website Stale labels Jul 19, 2024
Copy link

This issue is stale because it has been open for 1 month with no activity. Remove stale label or comment or this will be closed in 1 month.

@github-actions github-actions bot added the Stale label Aug 20, 2024
@jamengual jamengual removed the Stale label Aug 20, 2024
Copy link

This issue is stale because it has been open for 1 month with no activity. Remove stale label or comment or this will be closed in 1 month.

@github-actions github-actions bot added the Stale label Sep 21, 2024
@github-actions github-actions bot closed this Oct 22, 2024
@jamengual jamengual removed the Stale label Oct 22, 2024
@jamengual jamengual reopened this Oct 22, 2024
@github-actions github-actions bot added the go Pull requests that update Go code label Oct 22, 2024
Copy link
Contributor

@X-Guardian X-Guardian left a comment

Choose a reason for hiding this comment

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

Hi @ragne, just a couple of small changes and we can get this merged.

"pagelen": 10,
"size": 4,
"page": 1
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
}
}

Needs EOF LF

@@ -841,7 +841,7 @@ based on the organization or user that triggered the webhook.
```

Hide previous plan comments to declutter PRs. This is only supported in
GitHub and GitLab currently. This is not enabled by default. When using Github App, you need to set `--gh-app-slug` to enable this feature.
GitHub, GitLab and Bitbucket currently. This is not enabled by default. When using Github App, you need to set `--gh-app-slug` to enable this feature.
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you detail that for Bitbucket Cloud, the comments are deleted rather than hidden.

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 docs Documentation go Pull requests that update Go code provider/bitbucket waiting-on-response Waiting for a response from the user
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants