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 PR requirements to the docs #12

Merged
merged 3 commits into from
Jan 9, 2023

Conversation

laceysanderson
Copy link
Member

This PR adds PR requirements to the docs that are based on Tripal 3 and include the decision by the PMC in Issue #10.
This PR does not close Issue #10.

To Be reviewed by PMC + Tripal community:

Please let me know if you DO or Do NOT agree with the following additions:

  • PRs must be left unmerged for 3 weekdays to give core developers a chance to learn from each other and provide any feedback. Larger or particularly important/interesting PRs should be announced in the Slack #core-dev channel.
  • PRs must describe what they do and provide manual testing instructions.
  • If providing a PR review, please keep the following in mind:
    • Use of deprecated functions should be pointed out but do not block merge.
    • Limit your comments to merge blocking and deprecated functions. Any other concerns you have you can create an issue to document in the repository and assign it to yourself.
    • Concerns are merge blocking if they are examples of these guidelines not being followed (e.g. no automated testing) or if the functionality being added does not work as described or produces a WSOD.

Please check my wording here:

  • PRs that include new functionality must also provide Automated Testing.
    • A PR should not reduce the overall test coverage of the repository. Code Climate will comment on your PR with the total coverage in the repository and include the change caused by your PR. This change must not be negative.
  • PRs must pass all automated testing marked as "Required" at the bottom of the PR.

And as always, I welcome any + all feedback on these changes

These are the changes in full:
image

@laceysanderson laceysanderson added the TOPIC: Tripal Community Documentation on community contribution and structure label Oct 21, 2022
@laceysanderson laceysanderson linked an issue Oct 21, 2022 that may be closed by this pull request
Copy link
Member

@Ferrisx4 Ferrisx4 left a comment

Choose a reason for hiding this comment

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

The three day waiting period will be helpful, I like it. Sometimes it is easy to miss a change that gets merged right away.

@laceysanderson
Copy link
Member Author

laceysanderson commented Oct 31, 2022

I brought this up with @spficklin in the cofest today and this was his feedback:

  • we do want deprecated functions to be merge blocking (exception only for PRs relating to functionality for the first alpha).
  • the following sentence is confusing and we may want to remove it.

    Limit your comments to merge blocking and deprecated functions. Any other concerns you have you can create an issue to document in the repository and assign it to yourself.

Thanks! I'll make changes accordingly and then re-request review.

@laceysanderson
Copy link
Member Author

Ok, I made the changes Stephen mentioned back in October 🙈 so this can be merged now.

@laceysanderson laceysanderson merged commit 650a4cc into 4.x Jan 9, 2023
@laceysanderson laceysanderson deleted the 10-gradually-increasing-standards-for-testing branch January 9, 2023 22:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
TOPIC: Tripal Community Documentation on community contribution and structure
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants