Archived
15
23
Fork
You've already forked meta
6

[Governance] Need of criteria for PR disapproval #132

Open
opened 2023年01月26日 22:20:20 +01:00 by fsologureng · 4 comments
Contributor
Copy link

Starting in #124, this sub-discussion is related to defining whether is needed conditions to disapprove pull request in addition to established to approval.

The suggested criteria is:

For disapproval:

  • Arguments against or questioning the relevance of the proposal not unanswered by the proponent.
  • At least one review comment unresolved without arguments or changes in code.

The need I see is that insufficient approval votes are not reason enough to merge neither to deny the request because the community could be without available task force to do additional reviews or because there is no enough experts in the area. But in case a proponent didn't response a review or explicitly refuse to do it, then the proposition should be clearly disapproved.

An additional argument is that in human relations the truth is not as binary as in computing, so the disapproval criteria is not exactly the negation of the approval one.

Probably the redaction is not the best for the proposition. Feel free to suggest rephrases or addition of conditions, but the idea is to enforce:

  1. relevance (roadmap)
  2. technical guidelines (codebase)

(Is not under discussion that all the interaction is under strict conformance of CoC, so is not need to add clauses for that).

@dachary, @crystal, @KaKi87, please comment

Starting in #124, this sub-discussion is related to defining whether is needed conditions to disapprove pull request in addition to established to approval. The suggested criteria is: > For disapproval: > > - Arguments against or questioning the relevance of the proposal not unanswered by the proponent. > - At least one review comment unresolved without arguments or changes in code. The need I see is that insufficient approval votes are not reason enough to merge neither to deny the request because the community could be without available task force to do additional reviews or because there is no enough experts in the area. But in case a proponent didn't response a review or explicitly refuse to do it, then the proposition should be clearly disapproved. An additional argument is that in human relations the truth is not as binary as in computing, so the disapproval criteria is not exactly the negation of the approval one. Probably the redaction is not the best for the proposition. Feel free to suggest rephrases or addition of conditions, but the idea is to enforce: 1. relevance (roadmap) 2. technical guidelines (codebase) (Is not under discussion that all the interaction is under strict conformance of CoC, so is not need to add clauses for that). @dachary, @crystal, @KaKi87, please comment

@fsologureng you clearly articulate what is at stake when there is a need to close a PR. There could also be a number of other reasons such as decay (i.e. the content of the PR became obsolete over time) or redundancy (a competing PR was merged and rendered this one obsolete).

There is such a wide variety of reasons that I don't see how it would help to define criteria for closing a PR. If I go back to existing PRs I know about, I can't see when such criteria would have helped.

Do you see how I am struggling to find a use for such criteria?

@fsologureng you clearly articulate what is at stake when there is a need to close a PR. There could also be a number of other reasons such as decay (i.e. the content of the PR became obsolete over time) or redundancy (a competing PR was merged and rendered this one obsolete). There is such a wide variety of reasons that I don't see how it would help to define criteria for closing a PR. If I go back to existing PRs I know about, I can't see when such criteria would have helped. Do you see how I am struggling to find a use for such criteria?
Author
Contributor
Copy link

Do you see how I am struggling to find a use for such criteria?

mmm, maybe it would be better to define PR moderators criteria instead. For example, take in account at least the relevance and the technical guidelines as I tried to explaining here.

> Do you see how I am struggling to find a use for such criteria? mmm, maybe it would be better to define PR moderators criteria instead. For example, take in account at least the relevance and the technical guidelines as I tried to explaining here.

Are there existing Free Software projects with similar requirements? For comparison.

Are there existing Free Software projects with similar requirements? For comparison.
Author
Contributor
Copy link

Are there existing Free Software projects with similar requirements? For comparison.

In MDN for example, there is a mention in the Pull request submission and review guidelines, subsection Open a pull request:

If a pull request becomes too large, the reviewer may close it and ask that you to submit pull requests for each logical set of changes that belong together.

And, in the same page, but in the subsection Reviewing a pull request:

Close pull requests with unrelated changes if it is too complex or contains multiple unrelated changes. In such cases, ask the pull request author to submit their changes in smaller chunks.

> Are there existing Free Software projects with similar requirements? For comparison. In MDN for example, there is a mention in the [Pull request submission and review guidelines, subsection *Open a pull request*](https://developer.mozilla.org/en-US/docs/MDN/Community/Pull_requests#open_a_pull_request): > If a pull request becomes too large, the reviewer may close it and ask that you to submit pull requests for each logical set of changes that belong together. And, in the same page, but [in the subsection *Reviewing a pull request*](https://developer.mozilla.org/en-US/docs/MDN/Community/Pull_requests#reviewing_a_pull_request): > Close pull requests with unrelated changes if it is too complex or contains multiple unrelated changes. In such cases, ask the pull request author to submit their changes in smaller chunks.
Commenting is not possible because the repository is archived.
No Branch/Tag specified
readme
No results found.
Labels
Clear labels
[Decision] Building proposal(s)
We're in a decision-making process, buiding one or more proposals to address the shared aim based on the criteria
[Decision] Gathering criteria
We're in a decision-making process, gathering criteria, considerations and needs
[Decision] Integrating concerns
We're in a decision-making process, working with a proposal, trying to integrate concerns and create modifications/support such that the proposal works for everyone
Accessibility
Relates to Accessibility (a11y) of product, project and process.
Agreement proposal
Forgejo agreement proposal, following a discussion
Communication
Relates to all channels, social media, website, blog posts.
Election
Process of appointing a person into a role or team (if choosing people just for a specific one-time task, use the Entrustment label)
Entrustment
Process of choosing/approving specific people to do a critical/high-impact one-time task (if choosing people for an ongoing role/team, use the Election label)
Governance
Relates to processes, procedures and decision-making.
Meeting
An upcoming team meeting
User research - Accessibility
Requires input about accessibility features, likely involves user testing.
User research - Blocked
Do not pick as-is! We are happy if you can help, but please coordinate with ongoing redesign in this area.
User research - Community
Community features, such as discovering other people's work or otherwise feeling welcome on a Forgejo instance.
User research - Config (instance)
Instance-wide configuration, authentication and other admin-only needs.
User research - Errors
How to deal with errors in the application and write helpful error messages.
User research - Filters
How filter and search is being worked with.
User research - Future backlog
The issue might be inspiring for future design work.
User research - Git workflow
AGit, fork-based and new Git workflow, PR creation etc
User research - Labels
Active research about Labels
User research - Moderation
Moderation Featuers for Admins are undergoing active User Research
User research - Needs input
Use this label to let the User Research team know their input is requested.
User research - Notifications/Dashboard
Research on how users should know what to do next.
User research - Rendering
Text rendering, markup languages etc
User research - Repo creation
Active research about the New Repo dialog.
User research - Repo units
The repo sections, disabling them and the "Add more" button.
User research - Security
User research - Settings (in-app)
How to structure in-app settings in the future?
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
3 participants Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
forgejo/meta#132
Reference in a new issue
forgejo/meta
No description provided.
Delete branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?