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.

Sign up for GitHub

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

Jump to bottom

explicit patch submission instructions #180

Merged
merged 3 commits into from
Sep 26, 2024
Merged

explicit patch submission instructions #180

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9bfffa2
Update 07-lifecycle_of_a_Patch.Rmd
shannonpileggi Jul 22, 2024
d5888e5
Update 07-lifecycle_of_a_Patch.Rmd
shannonpileggi Jul 22, 2024
8a6c699
Merge branch 'main' into shannonpileggi-patch-1
llrs Sep 26, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 19 additions & 4 deletions 07-lifecycle_of_a_Patch.Rmd
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ Most often, changes are made to existing files, but if you happen to be adding a

This `patch.diff` file is the one that can be proposed to the R core via [Bugzilla](#SubmitPatches). You can also [ask for reviews](#PatchesReview) to the patch before proposing it to the R core via the [r-devel mailing list](https://stat.ethz.ch/mailman/listinfo/r-devel) or the slack channel of the R-contributors space.

### Using a git mirror
### Using a git mirror

Besides checking in your computer, you can use the Github mirror [r-devel/r-svn](https://github.com/r-devel/r-svn "A github svn mirror") of the source code to check this patch with different configurations and OS.

Expand Down Expand Up @@ -90,7 +90,7 @@ Once you are happy with the changes and the checks report that everything is oka

`https://patch-diff.githubusercontent.com/raw/r-devel/r-svn/pull/<pull_request_number>.diff`

With that file you can [submit your patch](#SubmitPatches), remember to check if it meets the [recommendations for good patches](#GoodPatches).
Save `<pull_request_number>.diff` as a plain text file to [submit your patch](#SubmitPatches), remember to check if it meets the [recommendations for good patches](#GoodPatches).

If you want to use `git` from the terminal to create the pull request (PR) to test the changes, you can use this [summary of the available git commands](https://about.gitlab.com/images/press/git-cheat-sheet.pdf "git cheat sheet").

Expand All @@ -112,9 +112,24 @@ When creating a patch for submission, there are several things that you can do t

## Submitting your patch for review {#SubmitPatches}

1. Patch in response to a pre-existing issue or bug report: In this case, you should attach the patch to the existing issue or bug report on Bugzilla with a brief comment.
### Patch in response to a pre-existing issue or bug report
Copy link
Member

Choose a reason for hiding this comment

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

I like how it is written.


2. Patch in response to an unreported issue or bug report: Assuming you already performed a search on Bugzilla for a pre-existing issue or bug and did not find the issue or bug reported, you need to create a new bug report and include your patch with it. Please fill in as much relevant detail as possible to prevent reviewers from having to delay reviewing your patch because of lack of information. Include (mostly as the first sentence), a to-the-point explanation of what the purpose of the patch is. This sentence should not be in the descriptive form, rather an imperative form will be more suitable here. If this is not enough detail for a patch, a new paragraphs(s) can be added to explain in proper depth what has happened. The details should be good enough that a core developer reading it understands the justification for the change.
In this case, you should attach the patch to the existing issue or bug report on Bugzilla with a brief comment.

1. Use the Attachments option to add the `*.diff` file as attachment.

2. On the new Create New Attachment web page, add a Brief Decription.

3. Select Content Type as `patch`.

4. Add comments (often prose text) in this page rather than in the original bugzilla page for the PR.

5. Press Submit. This will give you a bugzilla submission that sends one e-mail to all of R-core plus the PR author.


### Patch in response to an unreported issue or bug report

Assuming you already performed a search on Bugzilla for a pre-existing issue or bug and did not find the issue or bug reported, you need to create a new bug report and include your patch with it. Please fill in as much relevant detail as possible to prevent reviewers from having to delay reviewing your patch because of lack of information. Include (mostly as the first sentence), a to-the-point explanation of what the purpose of the patch is. This sentence should not be in the descriptive form, rather an imperative form will be more suitable here. If this is not enough detail for a patch, a new paragraphs(s) can be added to explain in proper depth what has happened. The details should be good enough that a core developer reading it understands the justification for the change.
Copy link
Member

Choose a reason for hiding this comment

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

This paragraph was moved from above, but I think we can improve it too in this PR (feel free to ignore the comments):

  1. Split each sentence in a different line: this will help future diffs to a single sentence.
  2. The content is related to other sections of the book, I would add links to them:
    a) To search on Bugzilla (section 3.3)
    b) How to report a bug (Chapter 4)
    c) How to write a good patch (7.4)

Reconsidering, perhaps splitting this in two sections complicates following the book. Adding a sentence at the end of the "Submitting your patch for review" section highlighting the differences on this PR (without the new third level section) and if it is all in one (open bug report+patch) might be easier...
What do you think?


## Getting your patch reviewed {#PatchesReview}

Expand Down