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

.github: add a "Contribution Guidelines" to the pull request template #46009

Merged
merged 2 commits into from Apr 27, 2022

Conversation

ljflores
Copy link
Contributor

@ljflores ljflores commented Apr 22, 2022

These guidelines refer contributors to the "Submitting Patches to Ceph" doc
and the "Submitting Patches to Ceph - Backports" doc. Even though there are
already tips for titling/signing commits in the PR template, these tips
are commented out and easy to gloss over once the contributor creates the
PR. These existing tips do not include any pointers about staging backports.

Fixes: https://tracker.ceph.com/issues/55418
Signed-off-by: Laura Flores lflores@redhat.com

Checklist

  • Tracker (select at least one)
    • References tracker ticket
    • Very recent bug; references commit where it was introduced
    • New feature (ticket optional)
    • Doc update (no ticket needed)
    • Code cleanup (no ticket needed)
  • Component impact
    • Affects Dashboard, opened tracker ticket
    • Affects Orchestrator, opened tracker ticket
    • No impact that needs to be tracked
  • Documentation (select at least one)
    • Updates relevant documentation
    • No doc update is appropriate
  • Tests (select at least one)
Show available Jenkins commands
  • jenkins retest this please
  • jenkins test classic perf
  • jenkins test crimson perf
  • jenkins test signed
  • jenkins test make check
  • jenkins test make check arm64
  • jenkins test submodules
  • jenkins test dashboard
  • jenkins test dashboard cephadm
  • jenkins test api
  • jenkins test docs
  • jenkins render docs
  • jenkins test ceph-volume all
  • jenkins test ceph-volume tox
  • jenkins test windows

These guidelines refer contributors to the "Submitting Patches to Ceph" doc
and the "Submitting Patches to Ceph - Backports" doc. Even though there are
already tips for titling/signing commits in the PR template, these tips
are commented out and easy to gloss over once the contributor creates the
PR. These existing tips do not include any pointers about staging backports.

Fixes: https://tracker.ceph.com/issues/55418
Signed-off-by: Laura Flores <lflores@redhat.com>
@github-actions github-actions bot added the CI Continuous Integration label Apr 22, 2022
@ljflores
Copy link
Contributor Author

@djgalloway I've noticed that some new contributors are still unsure about the commit/backport workflow. Not sure if you have any opinions on this suggestion, but let me know if it seems reasonable.

@ljflores
Copy link
Contributor Author

jenkins test make check

@@ -21,6 +21,11 @@
- The Signed-off-by line in every git commit is important; see <span class="x x-first x-last">[Submitting Patches to Ceph](https://github.com/ceph/ceph/blob/master/</span>SubmittingPatches.rst<span class="x x-first x-last">)</span>
-->

## Contribution Guidelines
- To sign and title your commits, please refer to [Submitting Patches to Ceph](https://github.com/ceph/ceph/blob/master/SubmittingPatches.rst).

Choose a reason for hiding this comment

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

Isn't this sort of just a repeat of line 21?

Copy link
Contributor Author

@ljflores ljflores Apr 26, 2022

Choose a reason for hiding this comment

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

Yeah, it is somewhat a repeat. However, line 21 is commented out, and many people miss it when they go to create their PR. The link in the comment is also not clickable. My suggestion is to move the link out the comment so that people are more likely to take notice of the link.

IMO these lines make sense to keep in comment form:

- Please give your pull request a title like
[component]: [short description]
- Please use this format for each git commit message:
[component]: [short description]
[A longer multiline description]
Fixes: [ticket URL on tracker.ceph.com, create one if necessary]
Signed-off-by: [Your Name] <[your email]>
For examples, use "git log".

But this should be removed and housed under the "Contribution Guidelines" section:

- The Signed-off-by line in every git commit is important; see <span class="x x-first x-last">[Submitting Patches to Ceph](https://github.com/ceph/ceph/blob/master/</span>SubmittingPatches.rst<span class="x x-first x-last">)</span>

Choose a reason for hiding this comment

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

Ah, I didn't know it was commented out.

Ideally, this should not be in comment form so users
can click on the link.

Signed-off-by: Laura Flores <lflores@redhat.com>
@ljflores ljflores added the skip-teuthology For PRs whose changes do not have an effect on QA runs/changes are not being tested in Teuthology label Apr 26, 2022
@ljflores ljflores merged commit 7ecbc83 into ceph:master Apr 27, 2022
@ljflores ljflores deleted the wip-link-submitting-backports branch May 31, 2022 01:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI Continuous Integration skip-teuthology For PRs whose changes do not have an effect on QA runs/changes are not being tested in Teuthology
Projects
None yet
2 participants