Skip to content

Pull Request Checklist

Alex Harsányi edited this page Aug 27, 2020 · 8 revisions

Thanks for submitting a pull request for the Racket plot package. The following is a list of items that need to be completed for the pull request to be merged in. If you are unsure about one or more items in this list, submit your pull request anyway, and we will help you to bring the pull request in a shape that can be merged.

  • Did you explain the reason for your changes?
  • Did you add implementation comments?
  • Does the build pass?
  • Have you documented all API changes?
  • Did you add history tags to the documentation changes?
  • Did you add new parameters to the parameter-group?
  • Did you add unit tests?

The following sections explain each point in more detail and if you have any questions or need help with any of these items, put a question on your pull request and we'll help you.

Did you explain the reason for your changes?

While it might seem obvious to you, it is not always clear to everyone why some changes are needed. When creating a pull request, consider explaining:

  • what the changes do? -- consider adding a picture illustrating the use of the new or updated features
  • why they needed? -- don't assume this is obvious to everyone,
  • what, if any alternatives exist? -- sometimes the same result can be achieved in a different way

Did you add implementation comments?

This is different from documenting the changes in the manual. The manual explains user level functionality. For all internal functions, consider adding comments explaining what the functions / methods / variables do and why are they doing it.

Originally the plot package was the work of a single person who was intimately familiar with it. The plot package is now maintained by people who don't have a complete understanding of the code base and receives contributions from people who know even less about this code base. We attempt to mitigate this by introducing developer comments explaining the internals, having an automated build and a test suite.

Does the build pass?

The plot package has an automated build set up, which will kick off when you create a pull request or push new changes to it. You can check the build results on the pull request page on GitHub.

Have you documented all API changes?

All user visible functions and changes to user visible functions need to be documented in the plot manual. The sources for the manual are found in the plot-doc package. If you need guidance, you can check for examples of similar things being documented, and there is a manual for the Scribble document format as well, plus a manual dedicated to writing documentation in Scribble

Did you add history tags to the documentation changes?

When new APIs are added or existing APIs are changed, the Scribble manual entry for them should contain a history entry, documenting the Racket version where the API was introduced and changed.

@history[#:added "7.8"]
@history[#:changed "7.9" "Added support for pictures for #:label"]

When adding history entries, use the next racket version as the version number (the plot package is shipped with Racket and its version will be meaningful to the users). Keep the changed entries brief -- they are intended for the end users, the developers can already look at the git history of the project.

Did you add new parameters to the parameter-group? (if applicable)

If your added new parameters to the plot package, and these parameters are used for controlling drawing (as opposed to supplying default arguments to renderers), you will need to add the new parameters to the parameter-groups.rkt file, so these parameters are saved and restored the plots are used interactively, either inside DrRacket REPL and using plot-frame.

You do not need to add your parameters to the parameter-groups.rkt file if these parameters are only used to supply default arguments to renderer functions.

Did you add unit tests?

All changes to have a corresponding test, which verifies the functionality. There are several examples in the plot-test folder of how to create tests for new or updated rendering functionality. Tests for other functionality (such as fixes) can also be written.

Tests are created as separate files in the plot-test package and simply placing a test file in a sub-folder of plot-test will make the test system pick it up, you don't need to register these test files.

The test file should be named after your pull request number (or issue number if fixing an issue) in the PR folder.

Clone this wiki locally