Skip to content

Conversation

wholmgren
Copy link
Member

@wholmgren wholmgren commented Jul 26, 2017

  • Closes issue add PULL_REQUEST_TEMPLATE.md file #354.
  • Fully tested. Added and/or modified tests to ensure correct behavior for all reasonable inputs. Tests must pass on the TravisCI and Appveyor testing services.
  • Code quality and style is sufficient. Passes git diff upstream/master -u -- "*.py" | flake8 --diff and/or landscape.io linting service.
  • New code is fully documented. Includes sphinx/numpydoc compliant docstrings and comments in the code where necessary.
  • Updates entries to docs/sphinx/source/api.rst for API changes.
  • Adds description and name entries in the appropriate docs/sphinx/source/whatsnew file for all changes.

Brief description of the problem and proposed solution (if not already fully described in the issue linked to above):

Adds a pull request template. I'm open to suggestions on how to improve it. I'll merge in a few days.

@wholmgren wholmgren added this to the 0.4.6 milestone Jul 26, 2017
@markcampanelli
Copy link
Contributor

Is there any conception of what a 1.0 release would entail?

@wholmgren
Copy link
Member Author

@thunderfish24 we have not seriously discussed what a 1.0 release would look like. Does it impact this PR? I didn't understand the point of your semver link on the related issue, so I feel like I must be missing something. I understand the semver concept and try to manage pvlib in a similar way, but I don't see the connection to the pull request template. The point of the PR template is to get contributors to check or at least think about a few things before the code reviewers comment on them. So, it seems to me that the PR template would be the same no matter what version we're on.

In any case, it would be a good idea to discuss a roadmap to a 1.0 release in a dedicated issue.

@markcampanelli
Copy link
Contributor

markcampanelli commented Jul 28, 2017

At Workiva we use a versioning checklist in our PR templates for post-1.0.0 repo's:

Semantic Versioning (check all that apply)

Patch

  • This change does not affect the public API

Minor

  • This change adds something to the public API
  • This change deprecates a part of the public API

Major

  • This change modifies part of the public API in a backwards-incompatible manner
  • This change removes part of the public API

@markcampanelli
Copy link
Contributor

We also use the rule of thumb that once you have more than one consumer, it's time to go >=1.0.0.

Personally, I think it's good to be a stickler about not breaking consumers' code "out of the blue", so that consumers should not have to worry about minor/patch bumps (other than new bugs!). Furthermore, "graceful" upgrade paths are great wherever possible: Deprecate with instructions on how to upgrade in a minor bump before removing deprecations in the next major bump. (One can always wait longer than the next major bump, if the need arises.)

@wholmgren
Copy link
Member Author

Thanks for clarifying. I had not considered that kind of checklist on a PR before. I think that could be useful once we get to 1.0.0, but I'd like to keep it a little more flexible until then. I'm a little surprised I don't see it on any of the major pydata projects that I've surveyed. I guess that could be because so much of pydata is sub-1.0.0.

I want to incorporate ideas from a few more templates before merging this PR:

@markcampanelli
Copy link
Contributor

Cool. I agree that updating the api.rst and whatsnew.rst seems sufficient at this time.

@wholmgren wholmgren modified the milestones: 0.5.0, 0.4.6 Aug 8, 2017
@wholmgren wholmgren merged commit 8e7253a into pvlib:master Aug 11, 2017
@wholmgren wholmgren deleted the prchecklist branch September 9, 2017 20:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants