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(reaction): disable reactions by setting an empty string #3360

Merged
merged 3 commits into from
May 6, 2023

Conversation

marcusramberg
Copy link
Contributor

This commit improves on that to make it possible to implement reactions in other VCS providers and also reuse the existing comment parsing.

what

My initial implementation of the reactions have a couple of problems:

  • The reaction happens inside the github block so it can't be implemented for other VCS providers
  • It re-implements comment parsing so it does not work for new supported calling forms like @bot-nick apply

why

This PR attempts to remedy the above problems by passing the comment ID to handleCommentEvent and doing
the reaction from there. The test written for the functionality continues to pass, indicating that react is
still being called :) I've also added a check to allow disabling reactions by setting them to the empty
string.

Other VCS providers can then be implemented by passing a comment id and implementing ReactToComment

tests

This refactor is tested through the unit test added in previous PR.

references

Initial reaction implementation in #2706

@marcusramberg marcusramberg requested a review from a team as a code owner April 29, 2023 13:23
@github-actions github-actions bot added the go Pull requests that update Go code label Apr 29, 2023
@marcusramberg marcusramberg changed the title Improve rcomment reaction support Improve comment reaction support Apr 29, 2023
GenPage
GenPage previously approved these changes May 1, 2023
Copy link
Member

@GenPage GenPage left a comment

Choose a reason for hiding this comment

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

Thank you for cleaning this up

…b, and only worked for 'atlantis' command.

This commit improves on that to make it possible to implement reactions in other VCS providers and also reuse the existing comment parsing.
@nitrocode nitrocode changed the title Improve comment reaction support feat(reaction): disable reactions by setting an empty string May 5, 2023
@nitrocode nitrocode added this to the v0.23.6 milestone May 5, 2023
@nitrocode
Copy link
Member

Were you also able to test in your setup with the reaction and without?

@marcusramberg
Copy link
Contributor Author

Were you also able to test in your setup with the reaction and without?

I tested the previous version, and it only added reactions for 'atlantis' and not @botnick, that's what motivated this PR. Here's it looks in our sandbox now:

image

@nitrocode nitrocode merged commit 4ed44d9 into runatlantis:main May 6, 2023
@nitrocode
Copy link
Member

nitrocode commented May 6, 2023

Thanks @marcusramberg ! We really appreciate your PRs. Looking forward to the next release!

For now, please use the dev release image to continue to test out this new feature prior to the next official release.

@GenPage GenPage modified the milestones: v0.23.6, v0.24.0 May 6, 2023
@jamengual
Copy link
Contributor

this is pretty cool thanks @marcusramberg again!!!

mtavaresmedeiros pushed a commit to mtavaresmedeiros/atlantis that referenced this pull request Jul 3, 2023
…ntis#3360)

* fix: Previous implementation tried to do comment parsing inside github, and only worked for 'atlantis' command.

This commit improves on that to make it possible to implement reactions in other VCS providers and also reuse the existing comment parsing.

* test: Add negative test to confirm reactions are not added for invalid comments

* test: Add a negative test to confirm invalid commands do not get a reaction
ijames-gc pushed a commit to gocardless/atlantis that referenced this pull request Feb 13, 2024
…ntis#3360)

* fix: Previous implementation tried to do comment parsing inside github, and only worked for 'atlantis' command.

This commit improves on that to make it possible to implement reactions in other VCS providers and also reuse the existing comment parsing.

* test: Add negative test to confirm reactions are not added for invalid comments

* test: Add a negative test to confirm invalid commands do not get a reaction
ijames-gc pushed a commit to gocardless/atlantis that referenced this pull request Feb 13, 2024
…ntis#3360)

* fix: Previous implementation tried to do comment parsing inside github, and only worked for 'atlantis' command.

This commit improves on that to make it possible to implement reactions in other VCS providers and also reuse the existing comment parsing.

* test: Add negative test to confirm reactions are not added for invalid comments

* test: Add a negative test to confirm invalid commands do not get a reaction
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
go Pull requests that update Go code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants