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

Updating PR template #161

Merged
merged 5 commits into from
Apr 5, 2022
Merged

Updating PR template #161

merged 5 commits into from
Apr 5, 2022

Conversation

jgalan
Copy link
Member

@jgalan jgalan commented Mar 24, 2022

jgalan Ok

This is a simple PR to update the PR message template that appears when creating a new PR.

Feel free to suggest changes now, the PR template is a relevant communication channel!

On the other hand, I believe we should generate a new version framework release once each PR has been approved.

2.3.10 --> X.Y.Z

X: Major change. For example, Migrating from ROOT6 to ROOT7. or from RML to YML.
Y: Official data will start to be produced. To define a point in time where a experiment/project started to take physics data.
Z: Any PR increases this number.

Changing version is basically based on 3 steps:

  1. Adding any new library releases that were generated since last framework release as a submodule,
  2. TRestVersion.h generation using a script,
  3. creating a Git tag, and
  4. placing the PR changes into the release notes.

@rest-for-physics/core_dev

@jgalan jgalan requested review from juanangp, lobis, nkx111 and a team March 24, 2022 09:36
Comment on lines 6 to 10
- Upgraded ...
- Pipeline validation ...
- Documented ...
- Example ...
- Script ...
Copy link
Member

Choose a reason for hiding this comment

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

I think you are missing other descriptions such as bug-fix, or feature. Would be a good idea to provide an example of a PR in every case. Also, you can point out to the different labels that are available, perhaps we should review the labels as well.

Copy link
Member Author

@jgalan jgalan Mar 25, 2022

Choose a reason for hiding this comment

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

I think that is contemplated at the upper line Fixes #

For me the - are points that the one writing the PR description should fill in. So this template is a guide/reminder of which points should be written. Not meaning that all of them should be filled.

Making it too long could be counter-productive.

About the labels, perhaps this could be defined at the contribution guide, then we review the labels on GitHub.

@jgalan jgalan requested a review from a team April 5, 2022 08:32
@jgalan
Copy link
Member Author

jgalan commented Apr 5, 2022

If you agree, could we approve this PR? If not, please, propose a change through a direct commit into this PR.

@jgalan
Copy link
Member Author

jgalan commented Apr 5, 2022

No, if the PR size is lower than 100, the PR size badge will show green, above 100 orange, and above 200 red.

@jgalan jgalan merged commit 2cb8b08 into master Apr 5, 2022
@jgalan jgalan deleted the jgalan_pr_template branch April 5, 2022 08:56
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.

None yet

3 participants