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

Contribution guidelines #3

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

stoobie
Copy link
Collaborator

@stoobie stoobie commented Aug 17, 2021

This PR is based on the Pantheon 2 repo README file.
@redhat-documentation/publishing-tutorial-team Please review.

@stoobie stoobie self-assigned this Aug 17, 2021
@stoobie stoobie added the needs review Someone needs to review technical correctness label Aug 17, 2021
Copy link

@domiborges domiborges left a comment

Choose a reason for hiding this comment

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

Hey @stoobie I completed the review. Overall it's very simple and action oriented. I just added few comments/suggestions.

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated
1. Remove `WIP` from the title of the pull request.
1. Click **Request Review** and enter `@@redhat-documentation/publishing-tutorial-team`.

The developers check that the **Tech review complete** label has been added to the pull request and peer pull request approval provided, then accept it.

Choose a reason for hiding this comment

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

I'm a bit confused with "peer pull request approval provided" It seems that you wanted to say something like "that the reviewer approved the pull request" or something similar.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah, actually, I don't think we need this same process here. I changed this section a bit.


### The review process

At least one person on the project team reviews the merge request and adds comments in GitHub. This review process will be open for one week from the day the merge request was submitted. If the merge request is still being actively discussed beyond the one week timeframe, then the merge request will stay open. Once the merge request discussion is resolved, the merge request will be NACK'd or ACK'd based on the comments given. If no comments are given after a week, then a project administrator will ACK the merge request.

Choose a reason for hiding this comment

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

Hey @stoobie the PR looks good. So do I understand correctly that as a reviewer I don't need to do anything? That if I don't object it's going to be merged in a week?

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

Successfully merging this pull request may close these issues.

2 participants