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

chore(workflow): Remove comment action from merge conflict labeler #136

Closed
wants to merge 1 commit into from

Conversation

bsmth
Copy link
Member

@bsmth bsmth commented Nov 15, 2023

Edit:
A 50/50 split of reactions after quickly testing the waters is enough for me to close for now while we consider other options moving forward.

Summary

We may want to disable comments on merge conflicts entirely. Opening a draft PR for early feedback.

Motivation

Some reviewers find that pull requests with many comments are an indicator of activity. If there are only bot comments, this may discourage people from reviewing changes as it appears a discussion is ongoing via PR comments.

related issues and pull requests:

We may want to disable comments on merge conflicts entirely. Opening a draft PR for early feedback.
@Josh-Cena
Copy link
Member

I find the comment being the entire point of the workflow, because it notifies the issue author or the reviewer that they need to update the branch. Issue labeling doesn't trigger a notification. The best world scenario would be the bot deleting the comment after the conflict is resolved, but that at least depends on eps1lon/actions-label-merge-conflict#40.

@OnkarRuikar
Copy link

If there are only bot comments, this may discourage people from reviewing...

This doesn't mean we start removing useful features.

Can we do following instead?

As soon as a PR is created we automatically assign a label Need Official Review. When a maintainer drops a review comment we automatically(or manually) remove the label, this can be done by watching for PR review comments in a workflow.
With this maintainers can use the label to filter PRs that haven't been officially reviewed.

@bsmth
Copy link
Member Author

bsmth commented Nov 17, 2023

This doesn't mean we start removing useful features.

I think whether it's useful is subjective 😄 but if it's useful for other reviewers, that's enough for me to get over it - I'm bringing this up again because it seems I'm not alone with the additional noise the comments create.

The best world scenario would be the bot deleting the comment after the conflict is resolved

This would be great, editing a floating / sticky comment or deleting it when the merge can be done cleanly is also a good option.

Can we do following instead? As soon as a PR is created we automatically assign a label Need Official Review.

Labels for indicating PR status is great, in my opinion, but Need Official Review is all PRs, so it seems redundant to add this to every PR. We don't have to make changes / decisions now, so I would close this PR and we could continue in a Discussion if you think it makes sense.

@bsmth bsmth closed this Nov 17, 2023
@bsmth bsmth deleted the merge-conflicts branch November 17, 2023 09:48
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.

None yet

3 participants