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.

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

Revise pull request template #4039

Merged
merged 3 commits into from
Jun 18, 2020
Merged

Revise pull request template #4039

merged 3 commits into from
Jun 18, 2020

Conversation

shoyer
Copy link
Member

@shoyer shoyer commented May 6, 2020

See below for the new language, to clarify that documentation is only necessary
for "user visible changes."

I added "including notable bug fixes" to indicate that minor bug fixes may not
be worth noting (I was thinking of test-suite only fixes in this category) but
perhaps that is too confusing.

cc @pydata/xarray for opinions!

  • Closes #xxxx
  • Tests added
  • Passes isort -rc . && black . && mypy . && flake8
  • Fully documented, including whats-new.rst for user visible changes
    (including notable bug fixes) and api.rst for new API

shoyer added 2 commits May 6, 2020 12:05
See below for the new language, to clarify that documentation is only necessary
for "user visible changes."

I added "including notable bug fixes" to indicate that minor bug fixes may not
be worth noting (I was thinking of test-suite only fixes in this category) but
perhaps that is too confusing.
@max-sixty
Copy link
Collaborator

I think that's reasonable

I was leaning to "everyone add their name", as it's a way of giving contributors credit, and some visible ownership — even for small changes which might be someone's first PR.

But I agree that's not always consistent with making the whatsnew better for readers.

@shoyer
Copy link
Member Author

shoyer commented May 6, 2020

I was leaning to "everyone add their name", as it's a way of giving contributors credit, and some visible ownership — even for small changes which might be someone's first PR.

I agree, this is a nice idea.

My goals were to make things simpler for new contributors and also to make whats-new a little bit for readers.

Two ways we might still achieve this:

  • We could still have a section in the release notes for each version for contributors to add their name
  • We could update this manually as part of the release process (but this adds one more step for core developers)

@max-sixty
Copy link
Collaborator

max-sixty commented May 7, 2020

* We could update this manually as part of the release process (but this adds one more step for core developers)

Now I think of it, we do this already:
https://github.com/pydata/xarray/blame/master/HOW_TO_RELEASE.md#L114

So maybe we paste that into the whatsnew too, and we're covered?

@dcherian
Copy link
Contributor

So maybe we paste that into the whatsnew too, and we're covered?

Seems like this was the consensus at this meeting. So let's add another step to HOW_TO_RELEASE and then merge this?

@shoyer
Copy link
Member Author

shoyer commented Jun 15, 2020

OK, revised.

Copy link
Contributor

@dcherian dcherian left a comment

Choose a reason for hiding this comment

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

Thanks @shoyer

@shoyer shoyer merged commit b9e6a36 into pydata:master Jun 18, 2020
dcherian added a commit to TomNicholas/xarray that referenced this pull request Jun 24, 2020
…o-combine

* 'master' of github.com:pydata/xarray: (81 commits)
  use builtin python types instead of the numpy alias (pydata#4170)
  Revise pull request template (pydata#4039)
  pint support for Dataset (pydata#3975)
  drop eccodes in docs (pydata#4162)
  Update issue templates inspired/based on dask (pydata#4154)
  Fix failing upstream-dev build & remove docs build (pydata#4160)
  Improve typehints of xr.Dataset.__getitem__ (pydata#4144)
  provide a error summary for assert_allclose (pydata#3847)
  built-in accessor documentation (pydata#3988)
  Recommend installing cftime when time decoding fails. (pydata#4134)
  parameter documentation for DataArray.sel (pydata#4150)
  speed up map_blocks (pydata#4149)
  Remove outdated note from datetime accessor docstring (pydata#4148)
  Fix the upstream-dev pandas build failure (pydata#4138)
  map_blocks: Allow passing dask-backed objects in args (pydata#3818)
  keep attrs in reset_index (pydata#4103)
  Fix open_rasterio() for WarpedVRT with specified src_crs (pydata#4104)
  Allow non-unique and non-monotonic coordinates in get_clean_interp_index and polyfit (pydata#4099)
  update numpy's intersphinx url (pydata#4117)
  xr.infer_freq (pydata#4033)
  ...
dcherian added a commit to raphaeldussin/xarray that referenced this pull request Jul 1, 2020
* upstream/master: (21 commits)
  fix typo in error message in plot.py (pydata#4188)
  Support multiple dimensions in DataArray.argmin() and DataArray.argmax() methods (pydata#3936)
  Show data by default in HTML repr for DataArray (pydata#4182)
  Blackdoc (pydata#4177)
  Add CONTRIBUTING.md for the benefit of GitHub
  Correct dask handling for 1D idxmax/min on ND data (pydata#4135)
  use assert_allclose in the aggregation-with-units tests (pydata#4174)
  Remove old auto combine (pydata#3926)
  Fix 4009 (pydata#4173)
  Limit length of dataarray reprs (pydata#3905)
  Remove <pre> from nested HTML repr (pydata#4171)
  Proposal for better error message about in-place operation (pydata#3976)
  use builtin python types instead of the numpy alias (pydata#4170)
  Revise pull request template (pydata#4039)
  pint support for Dataset (pydata#3975)
  drop eccodes in docs (pydata#4162)
  Update issue templates inspired/based on dask (pydata#4154)
  Fix failing upstream-dev build & remove docs build (pydata#4160)
  Improve typehints of xr.Dataset.__getitem__ (pydata#4144)
  provide a error summary for assert_allclose (pydata#3847)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants