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

Add ability to ignore certain users comments #1039

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

thekad
Copy link
Contributor

@thekad thekad commented Mar 18, 2024

Some build/CI systems can become too noisy, you still want to pull real people's comments and ignore bot accounts when adding annotations.

Some build/CI systems can become too noisy, you still want to pull real
people's comments and ignore bot accounts when adding annotations.

Signed-off-by: Jorge Gallegos <[email protected]>
Copy link
Collaborator

@ryneeverett ryneeverett left a comment

Choose a reason for hiding this comment

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

Implementation looks fine but these options need to be documented and tested.

@@ -151,6 +155,9 @@ def annotations(self, change):
else:
username = item['author']['_account_id']
# Gerrit messages are really messy
Copy link
Collaborator

Choose a reason for hiding this comment

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

This comment was about the cleaning of the message below. You should move it below your addition or, perhaps better, delete it.

@thekad
Copy link
Contributor Author

thekad commented Mar 19, 2024

Implementation looks fine but these options need to be documented and tested.

I'll add the test bits for github, there is no annotation testing done at all for gerrit but I can take a shot at it.

Question: Do you think it makes sense to leave these options on a per-service basis?

@ryneeverett
Copy link
Collaborator

I'll add the test bits for github, there is no annotation testing done at all for gerrit but I can take a shot at it.

if you can't test gerrit that's ok. Github is the critical one due to it's popularity and number of features.

Question: Do you think it makes sense to leave these options on a per-service basis?

Yes, I think we should leave them on a per-service basis. Unfortunately, it isn't a safe assumption that usernames are owned by the same entity (or in the case of bots, running the same software) across services and I don't think we should introduce that assumption here. For example, you might want to ignore a spammer who is username-squatting on one platform without ignoring the legitimate user on another platform.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants