Expanded PR section of code standards doc

[sseago]

Thank you for contributing to Velero!

Please add a summary of your change

Updates code standards doc with some additional content around PR best practices, as discussed in a recent community meeting.

Does your change fix a particular issue?

Fixes #(issue)

Please indicate you've done the following:

• [x ] Accepted the DCO. Commits without the DCO will delay acceptance.
• [ x] Created a changelog file or added /kind changelog-not-required as a comment on this pull request.
• Updated the corresponding documentation in site/content/docs/main.

[sseago]

/kind changelog-not-required

[weshayutin]

Nice.. Thanks @sseago

[mateusoliveira43]

since we have issue placeholder in PR template, I would discourage issue ref in PR title (unless this helps with any automation, like what we have for JIRA in OADP)

But I would also add a section explaining how to use the PR template, I saw a lot of misusage

maybe update it with comments to help users fill it (example https://raw.githubusercontent.com/dora-metrics/pelorus/master/.github/PULL_REQUEST_TEMPLATE/general_template.md)

[sseago]

While I don't want to require use of the issue in the title/commit message, I can see some value in including it, since PR template won't show up when looking at commit history. That's why I gave both options -- i.e. if you want the issue in title, that's fine, as long as there's also a description, but just a description is fine too.

[sseago]

The most important point (and the main reason for this change) is to make sure that every PR gets a descriptive title -- that "fix issue #foo" alone is insufficient.

[anshulahuja98]

@sseago unrelated to this PR going in, we can perhaps create github actions to ensure proper naming of PRs?

[codecov]

Codecov Report

Merging #6844 (cd7e2d6) into main (402703f) will increase coverage by 0.21%.
Report is 50 commits behind head on main.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main    #6844      +/-   ##
==========================================
+ Coverage   60.46%   60.67%   +0.21%     
==========================================
  Files         242      249       +7     
  Lines       26033    26476     +443     
==========================================
+ Hits        15740    16064     +324     
- Misses       9187     9268      +81     
- Partials     1106     1144      +38     

see 38 files with indirect coverage changes

[kaovilai]

Should add some guideline regarding title for cherry-pick PRs.

[anshulahuja98]

Should we perhaps have some automation in form of github actions which can do the manual work of Cherry Pick while also conforming to the title guidelines?

[kaovilai]

Like kubernetes prow /cherry-pick? would be cool.

[kaovilai]

https://prow.k8s.io/command-help#:~:text=/cherrypick%20%5Bbranch%5D

[sseago]

@anshulahuja98 @kaovilai Having the ability to cherry-pick via bot would add some options. Note that this sort of automated cherry-pick might require an additional github action (or manual intervention) if there is a changelog file, since the CP PR will have a new number, so the changelog file will need to be renamed. I think that sort of automation is probably out of scope for this PR though and should be considered as a follow-on action/investigation.

[kaovilai]

@sseago reopen #2663 to continue work on adding prow GH Actions.

[sseago]

@kaovilai well changelog filename check but also actually changing the filename, since cherry-picking the commit will have the wrong filename. When doing this manually, after cherry-picking the commit and creating the new PR you have to go in and change the filename to match the new PR number and amend the commit.

[kaovilai]

changing the filename

github actions can do that automatically :)

[sseago]

It will need to do it after creating the PR, though, since we don't know what the filename will be until the PR exists.

[kaovilai]

on: pr-comment
if: /cherry-pick <branch>
    cp -R . /tmp/cherry-pick
    change-file-name-to-next-pr|issue-number
    git commit --amend --no-edit --sign-off
    gh pr create --base <branch>

[kaovilai]

I think it would be good to add cherry-pick PR title guidelines.
#6881 with title Add 'orLabelSelector' for backup, restore command can be unclear at first glance that this is cherry-pick.

[sseago]

@kaovilai "I think it would be good to add cherry-pick PR title guidelines."

Something like this?
"Cherry-pick PR titles should be in the form 'cherry-pick [] ' "

[kaovilai]

Something like this?
"Cherry-pick PR titles should be in the form 'cherry-pick [] ' "

Sure.
Or <branch-name>: Original PR title

[sseago]

@kaovilai possibly with "CP" in there too? To distinguish between "this is a cherry-pick PR for a release branch" vs. "this is an original commit for a release branch"? Not sure it's necessary, but it gives a clue to a reviewer that if it's an original commit it probably warrants a closer review than a CP.

[kaovilai]

possibly with "CP" in there too?

sure

[shubham-pampattiwar]

/kind changelog-not-required

[sseago]

Updated in response to feedback