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: --discard-approval-on-plan to dismiss approvals when planning #2696

Merged
merged 9 commits into from
Dec 19, 2022

Conversation

secustor
Copy link
Contributor

@secustor secustor commented Nov 19, 2022

This PR implements logic to dismiss reviews of already approved PRs if atlantis plan is executed afterwards again.

Currently only Github is supported. If other platforms are used the flag will simply not have any affect.

E2E test https://github.com/secustor/renovate_terraform_lock_in_subdirectory/pull/14

Context: #1508

@secustor secustor marked this pull request as ready for review November 24, 2022 12:33
@secustor secustor requested a review from a team as a code owner November 24, 2022 12:33
@Fabianoshz
Copy link
Contributor

@secustor to implement this for gitlab we have to just implement the DiscardReviews function on server/events/vcs/gitlab_client.go?

@secustor
Copy link
Contributor Author

secustor commented Dec 1, 2022

@secustor to implement this for gitlab we have to just implement the DiscardReviews function on server/events/vcs/gitlab_client.go?

Yes, that's correct.

@Fabianoshz
Copy link
Contributor

Fabianoshz commented Dec 1, 2022

@secustor to implement this for gitlab we have to just implement the DiscardReviews function on server/events/vcs/gitlab_client.go?

Yes, that's correct.

Nice, I'll take a look at the gitlab implementation after this get merged. Might be useful for my use case.

@nitrocode nitrocode added the waiting-on-review Waiting for a review from a maintainer label Dec 17, 2022
cmd/server.go Outdated Show resolved Hide resolved
secustor and others added 2 commits December 17, 2022 23:57
Co-authored-by: PePe Amengual <jose.amengual@gmail.com>
@nitrocode nitrocode merged commit 01a9a5f into runatlantis:main Dec 19, 2022
@nitrocode
Copy link
Member

Thank you @secustor for the contribution! It's very much appreciated. Please propose more if you find another improvement.

@secustor secustor deleted the discard_approval_on_plan branch December 19, 2022 08:06
@nitrocode nitrocode added this to the 0.22.0 milestone Dec 23, 2022
@nitrocode nitrocode changed the title feat: dismiss approvals when planning feat: --discard-approval-on-plan to dismiss approvals when planning Dec 26, 2022
@mcantinqc
Copy link

I see the flag "DiscardApprovalOnPlanFlag", but it is no used anywhere as effect that feature is activate by default on the latest version (0.22.0) and there are no way to deactivate it.

@jamengual
Copy link
Contributor

jamengual commented Jan 3, 2023

it is by default not activated, look at the code in this PR all new features are usually disabled by default.
Please open an issue and detail your setup if you have issues.

@nitrocode
Copy link
Member

nitrocode commented Jan 3, 2023

Hmm after re-reviewing this PR I believe @mcantinqc is correct. There is a flag but i dont think the flag is checked unless I'm mistaken.

I agree with @jamengual , @mcantinqc please create a ticket.

cc @secustor thoughts on this?

Workaround for now @mcantinqc is to use v0.21.0

Edit: i added the ticket #2911

@secustor
Copy link
Contributor Author

secustor commented Jan 3, 2023

I have created a hotfix at #2913

@nitrocode
Copy link
Member

Looking for people to test the new dev image out before cutting a 0.22.1 release.

cc @mcantinqc @secustor

@Fabianoshz
Copy link
Contributor

Hey, I'll do some testing later today, I'm interested in implement this for gitlab too.

@jamengual
Copy link
Contributor

@mcantinqc sorry, I misread the code , you were right all along

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
waiting-on-review Waiting for a review from a maintainer
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants