Renovate: Improve PR Template For Contributors

[souravdasslg]

The current pull request template for contributors needs to be updated with a list of todo, to make sure contributors performing those before submitting it for review. Also, some additional data points may be collected to identify the type of PR, eg Braking/Non-Breaking, etc

Current PR Template

[souravdasslg]

@rarkins @ViceIce @JamieMagee
This is the updated PR template. What do you think?

<!--
    Before submitting a Pull Request, please ensure you have signed the CLA using this GitHub App:
    https://cla-assistant.io/renovateapp/renovate

    Please ensure `Allow edits from maintainers.` checkbox is checked
-->

## Description

Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. List any dependencies that are required for this change.

<!--If your PR closes the issue, use keyword Closes instead of Fixes -->

Fixes # (issue)

## Type of change

- [ ] Bug fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected)
- [ ] This change requires a documentation update

## How Has This Been Tested

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration

- [ ] Unit Test
- [ ] Integration Test

## Checklist

- [ ] My code follows the style guidelines of this project
- [ ] I have performed a self-review of my own code
- [ ] I have commented my code, particularly in hard-to-understand areas
- [ ] I have made corresponding changes to the documentation
- [ ] My changes generate no new warnings
- [ ] I have added tests that prove my fix is effective or that my feature works
- [ ] New and existing unit tests pass locally with my changes
- [ ] Any dependent changes have been merged and published in downstream modules

[viceice]

Maybe real tests against platform x should be added to test section

[rarkins]

<!--If your PR closes the issue, use keyword Closes instead of Fixes -->

AFAIK, those two are essentially identical when it comes to GitHub, and personally I've never differentiated between them. Usually Fixes would only make sense grammatically if it's a bug fix, so I prefer Closes because it captures more uses cases, including features.

Also, Closes # 123 is not the same as Closes #123 - only the second one works.

[rarkins]

Type of change

In theory this should be captured by the PR title commit type, e.g. fix, feat, refactor. Also I don't expect there to be very many people submitting a breaking change so I think we can optimize for not that case. It is indicated with BREAKING: as the last line prefix.

I think documentation updates yes/no is a separate question, e.g.

## Documentation (please tick one)

- [ ] I have updated the documentation, or
- [ ] No documentation update is required

[JamieMagee]

Couple of comments on specific points from the checklist:

My code follows the style guidelines of this project

I don't think this is necessary, as we have yarn prettier-fix which will do it for you, and fail the build if it doesn't follow prettier guidelines

My changes generate no new warnings

Not necessarily a bad thing. @ViceIce introduced changes which enabled more warnings!

New and existing unit tests pass locally with my changes

Same here. The build will fail, which is a better indication than a checkbox.

---

Overall I'm all for bringing more structure to PRs, but I think we should move as many of these requirements to the build or test command as possible, instead of having to remember more manual steps.

Also, in my opinion, making the checklist a lot longer might increase the barrier to contributing, and I don't think we have a _huge_ issue with low quality PRs right now.

[rarkins]

## How Has This Been Tested

This is a great idea. Perhaps:

I have verified these changes via:
- [ ] Code inspection only, or
- [ ] Newly added unit tests, or
- [ ] Unit tests + ran on a real repository

Please describe how confident *you* are and if there's any further testing you recommend.

[rarkins]

For the checklist, I lean towards not having this, because it adds some unnecessary work.

- [ ] My code follows the style guidelines of this project

This is captured by our linting results.

- [ ] I have performed a self-review of my own code

I don't think anyone today would not have done "self-review", so it's kind of redundant.

- [ ] I have commented my code, particularly in hard-to-understand areas

It's a good idea, but I wonder if we can capture it elsewhere. e.g. part of our contributing guide. I want to be clearer that we need code to be as "understandable as possible", and not "as clever or minimal as possible". e.g. use long variable names that convey meaning, add commenting if code cannot be made understandable on its own, etc.

- [ ] I have made corresponding changes to the documentation

Captured earlier.

- [ ] My changes generate no new warnings

We don't enforce that now, and generally go for lint errors not warnings. I think we can skip this.

- [ ] I have added tests that prove my fix is effective or that my feature works
- [ ] New and existing unit tests pass locally with my changes

Testing has been captured in earlier section, and our CI will indicate if the tests pass or not :)

- [ ] Any dependent changes have been merged and published in downstream modules

Not sure what this is for?

[rarkins]

BTW putting this into PR form would make it much easier to review. I probably should have suggested that before reviewing! Please raise a PR so we can improve it there.

[souravdasslg]

Great. Will raise a PR soon with all these feedbacks 😁

[HonkingGoose]

@rarkins I noticed this was marked Done on the Renovate project board, but this issue is still open on the GitHub issue tracker. What do you want here? Do you want a pull request from me with the proposed changes, or do you want to close this issue and leave the pull request template as it is now?

Pull request template history: https://github.com/renovatebot/renovate/commits/master/.github/pull_request_template.md

[rarkins]

@HonkingGoose I think the Done was by mistake and we could still make it better, so I have marked it as "Ready".

FYI I have triggered a couple of invites for you just now, one for triage access to this repo (let's you help manage issues and PRs) as well as the Renovate "Project", which would let you add issues to it which are missing from the Project and also to update stages like this. Thank you for your contributions so far, I hope these permissions mean you can get things done with less inconveniences.

[renovate-release]

:tada: This issue has been resolved in version 23.61.6 :tada:

The release is available on:

• GitHub release
• 23.61.6

Your semantic-release bot :package::rocket: