-
-
Notifications
You must be signed in to change notification settings - Fork 37
Pull Request Checklist
Thanks for submitting a pull request for the Racket plot package. This document lists some items that need to be completed for your pull request to be merged -- this will help us keep the code base maintainable. If you are unsure about any item in this list, submit your pull request anyway, and we will help you out.
Completing these items does not imply that your changes will be accepted, this depends on whether they are appropriate for the plot package. If in doubt, and before you start making changes to the plot package, discuss your ideas either by creating an issue in this project, posting on the Racket Discourse group or on the Racket Discord server.
Here is the list. Have a look at the sections below for more detailed explanations about these questions:
- Did you explain the reason for your changes?
- Did you consider both 2D and 3D plots?
- Did you add implementation comments?
- Is the code correctly indented?
- 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?
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 are the changes needed? -- don't assume this is obvious to everyone, consider explaining why these features are useful
- what, if any, alternatives exist? -- sometimes the same result can be achieved in a different way, this shows that you considered the options.
The plot package supports both 2D and 3D plots and some features (for example various renderers) make sense to be added to both types of plots.
This is different from documenting the changes in the manual, as 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. This may be obvious to you now, but it might not necessarily be to other people and even to yourself when you revisit the code several months later.
Originally the plot package was the work of a single person who was intimately familiar with it, but it 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 limited knowledge by introducing developer comments explaining the internals, having an automated build and a test suite.
Make sure the code has a neat layout and is properly indented. For guidance, check out the Textual Matters section of the Racket Style Guide. If you use DrRacket or racket-mode, the they should handle indentation correctly, but mistakes can happen when the code is copied. Also note that the Racket project prefers a line length of 102 characters.
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.
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. 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 Racket documentation
When new APIs are added or existing APIs are changed, the manual entry for them should contain a history entry, documenting the Racket version where the API was introduced and changed. Here is an example for an "added" and "changed" history entry:
@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, as they are intended for the end users and the developers can already look at the git history of the project.
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.
All changes to have a corresponding test, which verifies the functionality introduced by the pull request. Your pull request should add new test cases, but it should not modify existing ones. If the existing test cases pass and they are unchanged, this is a good indication that your changes do not affect the existing rendering and are therefore backwards compatible.
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.
- create a
test-suitefor your pull request and use atest-casefor each test, the test suite should be run usingrun-testsfrom within a test module. - place the tests in a separate file in the
plot-testpackage -- the test suite will pick up the file automatically and run the tests - preferably, the file should be named after the pull request or issue resolved and placed in the PR folder.