Add a pull request template to the xwiki-platform Github

Development
#1

Hello! In order to facilitate code review, I decided to use a pull request template for my PRs (e.g. XWIKI-15065, XWIKI-20435). I think this is a practice that could benefit any PR (especially for those created by non-committers which will systematically go through a validation step) and that it’d be interesting to
incentivize using a common pattern that provides details on the PR.

My proposal is split in two points:

Here is a proposal of template:

# Jira
<!--- Always put a JIRA issue reference in a commit message --->

# PR Changes
<!--- Describe the main changes brought in this PR. --->
*

# Notes
<!--- Provide extra hints to make it easier to understand the PR. Those can be:
* Explanation of choices made in this PR
* Expected backporting/cherry-picking strategy
* Anchor towards extra resources needed to understand the context of this PR. E.g. a proposal on the forum.
--->
*

# View
<!--- If this PR introduces any UI change, it's recommended to highlight it with a screenshot or screen recording. --->

# Tests
<!--- Describe how the changes provided in this PR influence test results. Especially important for regression fixes. --->

This would become on GitHub dark mode (empty then filled up template):
prTemplateEmpty
prTemplateFull

Of course the categories and their order would be just a recommendation and not a strict requirement.

<EDIT> This template has been updated in regards to the feedback in this discussion and is available on post n.5. </EDIT>

Do you agree with the main idea of this proposal? Do you think this template is appropriate for most PRs on xwiki-platform ? Should we add a section or remove one that is too specific?

Thanks,
Lucas C.

#2

Looks good in general. Of course there are cases where the View is not needed, but it’s OK.

Just not sure about the Tests section, but I guess it’s a good passive-aggressive way to remind people that providing/updating related tests is always a good idea.

I see no reason to not do the same thing for xwiki-commons and xwiki-rendering (but maybe without the View section since it’s probably pretty rare in those cases).

1 Like
#3

+1 to have a template (and as Thomas said, we could have it on common, rendering, and contrib maybe too). The proposed one looks good for a start, we can always improve it over time.

#4

Sounds good. I’d change the following:

  • Jira → Jira URL
  • PR Changes → Description of changes
  • Remove Notes as it can go in the desc of changes (I don’t see the point of having 2 sections)
  • View → Screenshots & Video
  • Also indicate in comments for Screenshots & Video that before/after screenshots are highly recommended.
  • Tests → Executed Tests
  • Indicate in the comments for Executed Tests that the user should indicate how they tested the changes and especially what maven commands they’re ran to validate it

Thanks

#5

I agree with the changes you proposed, except for the removal of the Notes section. The main question answered in those two sections are different:

  • Description of changes answers What is changed ?
  • Notes answers Why is it changed like that ?

The title Notes does not really explain any of it but I did not really question it so far. Maybe Clarifications or Explanations would be better suited.

The * Expected backporting/cherry-picking strategy part in here does not fit with the above question. I think it’d be best to move it to a new section.

Here is the updated template proposal:

# Jira URL
<!--- Always put a JIRA issue reference in a commit message --->

# Changes
## Description
<!--- Describe the main changes brought in this PR. --->
*
## Clarifications
<!--- Provide extra hints to make it easier to understand the PR. Those could be:
* Explanation of choices made in this PR
* Anchor towards extra resources needed to understand the context of this PR. E.g. a proposal on the forum.
--->
*
# Screenshots & Video
<!--- If this PR introduces any UI change, it's recommended to highlight it with before/after screenshots or even a screen recording for complex interactions. --->

# Executed Tests
<!--- Especially important for regression fixes. 
Indicate how changes were tested, e.g. what maven commands were run to validate them.
--->
# Expected merging strategy
* Prefers squash: Yes / No
* Backport on branches:
  *

visuals of the above template in Github dark theme

I think it looks sharper, thanks a lot for the vocabulary changes propositions :slight_smile:

#6

That’s a great idea, +1 for the template!

A small note: I wouldn’t use comments in the markdown template, but I’d put directly the content of the comments in each section: it’s immediately visible so you cannot miss something. Check for example the template we’re using for security advisory: https://dev.xwiki.org/xwiki/bin/view/Community/SecurityPolicy/#HSecurityAdvisorytemplateandinformation

1 Like
#7

+1, sounds good. Regarding comments, I think using comments in such templates is a common practice - it avoids that you need to delete the instructions when creating the PR, thus reducing work, reducing cases where the instructions are part of the final PR and allowing to easily keep the instructions while you’re adding your content for reference.

1 Like
#8

Note: I’m planning to apply the template on Github tomorrow.

1 Like
#9

Done on xwiki-commons, xwiki-rendering, and xwiki-platform.
Do we also want to add a template in GitHub - xwiki-contrib/github-template-repository: Template GitHub repository for creating new contrib repos ?
If yes, do we want to add it to all contrib projects or do we give the choice to maintainers of existing projects?

1 Like
#10

IMO: All (it’s simplest and by default contrib projects are supposed to follow dev.xwiki.org practices - and exceptionally stray away from some on a case by case basis). See https://contrib.xwiki.org/xwiki/bin/view/Main/WebHome#HDevelopmentPractices

Thx

#11

I think it’s good to have a similar template in contrib projects, yes. It’s good that the whole community get used to the same suggestions.