Skip to content

Conversation

@maxrjones
Copy link
Member

Description of proposed changes

Adds some information based on comments raised during the release process for v0.4.0.

Fixes #

Reminders

  • Run make format and make check to make sure the code follows the style guide.
  • Add tests for new features or tests that would have caught the bug that you're fixing.
  • Add new public functions/methods/classes to doc/api/index.rst.
  • Write detailed docstrings for all functions/methods.
  • If adding new functionality, add an example to docstrings or tutorials.

Slash Commands

You can write slash commands (/command) in the first line of a comment to perform
specific operations. Supported slash commands are:

  • /format: automatically format and lint the code
  • /test-gmt-dev: run full tests on the latest GMT development version

@maxrjones maxrjones added documentation Improvements or additions to documentation skip-changelog Skip adding Pull Request to changelog labels Jun 22, 2021
@seisman seisman added this to the 0.5.0 milestone Jun 23, 2021
@seisman seisman added the final review call This PR requires final review and approval from a second reviewer label Jun 23, 2021
Copy link
Member

@weiji14 weiji14 left a comment

Choose a reason for hiding this comment

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

Thanks Meghan for doing a post-release review of PyGMT's release processes! Just one broken link to fix and a suggestion on the forum post announcement part.

modules, gallery examples, API docs changes) and entries within each group
are alphabetical.
6. Move a few important items from the main sections to the highlights section.
5. Edit the list of people who contributed to the release, linking to their
Copy link
Member

Choose a reason for hiding this comment

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

See #1341 (comment):

How do I know whether any of the current order reflects a case-by-case basis or whether I should update based on git shortlog -sne?

See previous discussion at #1047 (comment). Will leave it up to you to take a first stab at the order :) Just to clarify, that line refers to the Zenodo citation order. The release note authors order should go strictly by git shortlog (and remove the bots).

OK, I guess I wasn't paying good attention for that release :) Thanks for the note.

Just on the author list, this is the order from git shortlog -sne on the master branch (truncated to be for authors with >2 commits, and I've merged the counts for people using different emails). Note that the counts don't reflect co-authored commits properly, and should be taken as a rough guide:

Shall we also explicitly indicate that we should try to reflect co-authored commits?

Copy link
Member Author

Choose a reason for hiding this comment

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

I am glad to add something about review efforts and team programming, but I would prefer to not base this off co-authored commits. The co-authored commits method seems flawed to me because a review comment that leads to a local commit by the PR author can be equally or more helpful that suggestions that are committed on the GitHub interface and this method does not value a thorough review that does not lead to any comments.

I learned recently that you can manually add co-authors to your local commits that are recognized by GitHub by putting Co-authored-by: Name <[email protected]> but since this is not in the contributing guide not everyone is probably aware of the option, plus it is time consuming and easy to forget.

Copy link
Member Author

Choose a reason for hiding this comment

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

Please see if you are OK with the changes in ff27ac5

Copy link
Member

Choose a reason for hiding this comment

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

Please see if you are OK with the changes in ff27ac5

I didn't mean the authorship on Zenodo archives, which is okay to me. I mean to reflect co-authors in the release note, i.e., changelog. For example, a team of four contributors contributes to a PR, but only one owns the commit. Does this case exist? If no, please forget this comment.

Copy link
Member

Choose a reason for hiding this comment

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

I mean to reflect co-authors in the release note, i.e., changelog.

I don't think there is an easy way to automatically list "co-authors" in the changelog.

Copy link
Member

Choose a reason for hiding this comment

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

I tried in #1341 (comment) using git log --format="%(trailers:key=Co-authored-by)" to get co-author names, but 1) the output is long and messy and 2) it doesn't give counts. Maybe we could find a better way?

Copy link
Member

Choose a reason for hiding this comment

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

All the changes look good to me. We might find a better way to get co-authors' names later.

Copy link
Member

@michaelgrund michaelgrund left a comment

Choose a reason for hiding this comment

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

After fixing the proposed changes, this PR looks good to me!

Co-authored-by: Wei Ji <[email protected]>
Co-authored-by: Yao Jiayuan <[email protected]>
Co-authored-by: Dongdong Tian <[email protected]>
@seisman
Copy link
Member

seisman commented Jun 24, 2021

@meghanrjones I think this PR is now ready for merge.

@maxrjones maxrjones merged commit 006b44a into master Jun 24, 2021
@maxrjones maxrjones deleted the maintenance-release branch June 24, 2021 21:02
@seisman seisman removed the final review call This PR requires final review and approval from a second reviewer label Jun 24, 2021
@seisman seisman modified the milestones: 0.5.0, 0.4.1 Jul 25, 2021
@weiji14 weiji14 mentioned this pull request Aug 8, 2021
23 tasks
sixy6e pushed a commit to sixy6e/pygmt that referenced this pull request Dec 21, 2022
…nericMappingTools#1346)

Update release_checklist.md, AUTHORSHIP.md, and maintenance.md

Co-authored-by: Wei Ji <[email protected]>
Co-authored-by: Yao Jiayuan <[email protected]>
Co-authored-by: Dongdong Tian <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation skip-changelog Skip adding Pull Request to changelog

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants