Skip to content

Conversation

@themr0c
Copy link
Contributor

@themr0c themr0c commented Jun 24, 2021

chore: updating pull request template

What does this PR do?

  • Suggest the usage of a prefix for pull requests titles.
  • Distinguish automated and non automated mandatory steps, and in depth review checklist.
  • Clarify and complete the review checklist in the pull request template.
  • Add a document describing the complete code review checklist.
  • Fix language suggestions.

What issues does this PR fix or reference?

The review checklist contains not enough information.

Specify the version of the product this PR applies to

che-docs

Pull Request Checklist for committers and reviewers

The author and the reviewers validate the content of this Pull Request with this checklist, in addition to the automated tests.

  • Any procedure:
    • Successfully tested.
    • Describes a scenario already covered by QE tests, otherwise Che QE team has acknowledged an issue.
  • Any page or link rename:
  • Builds on Eclipse Che hosted by Red Hat.
  • Validate language on files added or modified step reports no vale warnings.

@themr0c themr0c requested review from a team, MichalMaler and rkratky as code owners June 24, 2021 13:42
@themr0c themr0c requested a review from ibuziuk June 24, 2021 13:42
@themr0c themr0c added this to the 7.33.x milestone Jun 24, 2021
@themr0c themr0c changed the title chore: update pull request template, chore: update pull request template Jun 24, 2021
Copy link
Member

@ibuziuk ibuziuk left a comment

Choose a reason for hiding this comment

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

IMO the 28 items checklist split into 3 levels is overkill.
Can we come up with smth simple + automated as much as possible?

Also, let's make a link to workspaces.openshift.com / 'Try it on Eclipse Che hosted by Red Hat ' a requirement for easy verification e.g. - #1986

@themr0c
Copy link
Contributor Author

themr0c commented Jun 28, 2021

Feedback from Che Community call:

  • The checklist belongs to the CONTRIBUTING.adoc page.
  • Better wording for the "Levels" titles.

@themr0c themr0c changed the title chore: update pull request template chore: update pull request template, add code review checklist Jun 29, 2021
@themr0c themr0c requested a review from ibuziuk June 29, 2021 07:46
@themr0c
Copy link
Contributor Author

themr0c commented Jun 29, 2021

@ibuziuk do you have an example how to write a link to the PR to create in a PR template? It is "easy" to add afterwards when the PR exists, and we have a link, but I don't really see how to achieve that in the template.

Copy link
Contributor

@mmorhun mmorhun left a comment

Choose a reason for hiding this comment

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

I completely agree that docs quality should be good, but in my opinion suggested list of requirements is too much and imprecise in some places.
Anyway, thanks to the docs team for working towards better docs and contribution process!

@themr0c themr0c requested a review from ibuziuk August 17, 2021 08:19
@themr0c themr0c modified the milestones: 7.33.x, 7.34.x Aug 17, 2021
@themr0c
Copy link
Contributor Author

themr0c commented Aug 17, 2021

All suggestions applied. @ibuziuk @pmkovar @mmorhun can you please give a go for another round of reviews?

@themr0c themr0c requested a review from mmorhun August 20, 2021 08:27
Copy link

@rkratky rkratky left a comment

Choose a reason for hiding this comment

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

@themr0c, nitpick: can we choose between "Pull Request" and "pull request", and only use one? (I vote for the latter.)

@themr0c themr0c merged commit ded1c28 into eclipse-che:master Aug 24, 2021
@themr0c themr0c deleted the chore-updating-pr-template branch August 24, 2021 18: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.

7 participants