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

Fix changelog authors #4372

Merged
merged 23 commits into from
Dec 21, 2023
Merged

Fix changelog authors #4372

merged 23 commits into from
Dec 21, 2023

Conversation

IAlibay
Copy link
Member

@IAlibay IAlibay commented Dec 17, 2023

Overview of changes & rationale

  1. Removed duplicate mattwthompson in 0.20.0 changelog entry.

mattwthompson was placed twice by accident, this removes this duplication.

  1. Addition of missing authors.

An retrospective analysis of the authors found via git shortlog -s -n --email --branches="develop" found several commits by authors which were not present in the AUTHORS.md file.

There seemed to be no indications in the above mentioned PRs that the author did not want to be in the authors file, it looks like it was just forgotten.

  1. Addition of missing entries from the changelog

Continued cross referencing of the git shortlog output but also accounting for the changelog identified several individuals that had not been included in the changelog entries for the release they contributed under. They were added to the relevant entry of the changelog based on the merge commit date.

Please note that for Tone Bengsten, we a) had no github handle (so they were assigned tone.bengsten), and b) no specific commit. Given we know that this individual was including alongside the encore merge, they were assigned to the 0.16.0 release.

Acknowledgements

Thanks to @fiona-naughton for helping out with digging into this data :)

PR Checklist

  • Tests?
  • Docs?
  • CHANGELOG updated?
  • Issue raised/referenced?

Developers certificate of origin


📚 Documentation preview 📚: https://mdanalysis--4372.org.readthedocs.build/en/4372/

@IAlibay
Copy link
Member Author

IAlibay commented Dec 17, 2023

@RMeli looks like the azure trigger works on draft PRs now! We should be good to be stringent on forcing draft over WIP.

Copy link

github-actions bot commented Dec 17, 2023

Linter Bot Results:

Hi @IAlibay! Thanks for making this PR. We linted your code and found the following:

There are currently no issues detected! 🎉

Copy link

codecov bot commented Dec 17, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (b808ef1) 93.41% compared to head (93347cf) 93.41%.

Additional details and impacted files
@@            Coverage Diff             @@
##           develop    #4372     +/-   ##
==========================================
  Coverage    93.41%   93.41%             
==========================================
  Files          171      185     +14     
  Lines        22512    23626   +1114     
  Branches      4129     4129             
==========================================
+ Hits         21029    22070   +1041     
- Misses         963     1036     +73     
  Partials       520      520             

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@IAlibay IAlibay marked this pull request as ready for review December 18, 2023 02:23
@IAlibay
Copy link
Member Author

IAlibay commented Dec 18, 2023

This one might be a bit contentious, so I'm going to tag @orbeckst who might know the most history / proper etiquette for adding folks.

@IAlibay IAlibay requested a review from orbeckst December 18, 2023 02:24
Copy link
Member

@hmacdope hmacdope left a comment

Choose a reason for hiding this comment

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

LGTM in terms of pure text etc.

Copy link
Member

@orbeckst orbeckst left a comment

Choose a reason for hiding this comment

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

Looks like a ton of work!

Could you please briefly summarize approach and rationale for the changes so that there is a public record of our approach? -- not every one in detail but in aggregate: Why are they being added retroactively and how did you decide who needed to be added?

I'd then want to include your statement in the final merge log message.

Thanks.

@orbeckst orbeckst self-assigned this Dec 18, 2023
@IAlibay
Copy link
Member Author

IAlibay commented Dec 18, 2023

@orbeckst I've added a short overview in the initial PR comment. Happy to add more details if necessary.

@orbeckst
Copy link
Member

orbeckst commented Dec 20, 2023

Scratch for commit message

Update of AUTHORS and CHANGELOG with inferred author contributions. 

# Overview of changes & rationale

*  Removed duplicate mattwthompson in 0.20.0 changelog entry.: mattwthompson was placed twice by accident, this removes this duplication.

* Addition of missing authors.

   An retrospective analysis of the authors found via `git shortlog -s -n --email --branches="develop"` found several commits by authors which were not present in the `AUTHORS.md` file.

    - Zhenbo Li: commited via PR: Started tests for gnm. #803 and Make Travis run tests on OSX. #771, 
    - Jenna M. Swarthout via PR Update CoC according to suggestions from current CoC committee #4289 and Point to new reporting form link (owned by [email protected]) #4298, 
    - Bradley Dice via PR   Fix GSD Windows compatibility #2384 , 
    - David Minh via PR #2668

   There seemed to be no indications in the above mentioned PRs that the author did not want to be in the authors file, it looks like it was just forgotten.

* Addition of missing entries from the changelog

   Continued cross referencing of the git shortlog output but also accounting for the changelog identified several individuals that had not been included in the changelog entries for the release they contributed under. They were added to the relevant entry of the changelog based on the merge commit date.

   Please note that for Tone Bengsten, we a) had no github handle (so they were assigned @tbengtsen), and b) no specific commit. Given we know that this individual was including alongside the encore merge, they were assigned to the 0.16.0 release.

# Detailed commit changes

* Update CHANGELOG
* PR #1218
* PR #1284 and #1408
* PR #4109
* based on git history
* PRs #803 and #771 (author addition, changelog addition)
* PR #2255 and #2221
* PR #1225
* PR #4289 and #4298
* PR #4031
* PR #4085
* PR #3635
* PR #2356
* PR #2559
* No GH handle - Encore author addition @tbengtsen 
* PR #4184
* PR #2614
* PR #2219 
* PR #2384
* PR #2668
* Add missing entry for Jenna

# Acknowledgements

Thanks to @fiona-naughton for helping out with digging into this data :)

Copy link
Member

@orbeckst orbeckst left a comment

Choose a reason for hiding this comment

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

Small change in guessing Tone Bengtsen's GH handle

package/CHANGELOG Outdated Show resolved Hide resolved
@orbeckst
Copy link
Member

@IAlibay can you please have a quick look at my suggested change, just need a second pair of eyes.

Given Fiona's help here, I will also add her as a co-contributor on the squash-merged PR.

Co-authored-by: Oliver Beckstein <[email protected]>
@IAlibay
Copy link
Member Author

IAlibay commented Dec 20, 2023

@IAlibay can you please have a quick look at my suggested change, just need a second pair of eyes.

Given Fiona's help here, I will also add her as a co-contributor on the squash-merged PR.

Looks good to me, thanks @orbeckst !

@orbeckst orbeckst merged commit 38d0dea into develop Dec 21, 2023
22 of 23 checks passed
@orbeckst
Copy link
Member

crap, I pressed the wrong button without adding the message.

Will have to do a force push on develop.

orbeckst added a commit that referenced this pull request Dec 21, 2023
Update of AUTHORS and CHANGELOG with inferred author contributions.

*  Removed duplicate mattwthompson in 0.20.0 changelog entry.: mattwthompson was placed twice by accident, this removes this duplication.

* Addition of missing authors.

   An retrospective analysis of the authors found via `git shortlog -s -n --email --branches="develop"` found several commits by authors which were not present in the `AUTHORS.md` file.

    - Zhenbo Li: commited via PR: Started tests for gnm. #803 and Make Travis run tests on OSX. #771,
    - Jenna M. Swarthout via PR Update CoC according to suggestions from current CoC committee #4289 and Point to new reporting form link (owned by [email protected]) #4298,
    - Bradley Dice via PR   Fix GSD Windows compatibility #2384 ,
    - David Minh via PR #2668

   There seemed to be no indications in the above mentioned PRs that the author did not want to be in the authors file, it looks like it was just forgotten.

* Addition of missing entries from the changelog

   Continued cross referencing of the git shortlog output but also accounting for the changelog identified several individuals that had not been included in the changelog entries for the release they contributed under. They were added to the relevant entry of the changelog based on the merge commit date.

   Please note that for Tone Bengsten, we a) had no github handle (so they were assigned @tbengtsen), and b) no specific commit. Given we know that this individual was including alongside the encore merge, they were assigned to the 0.16.0 release.

* Update CHANGELOG
* PR #1218
* PR #1284 and #1408
* PR #4109
* based on git history
* PRs #803 and #771 (author addition, changelog addition)
* PR #2255 and #2221
* PR #1225
* PR #4289 and #4298
* PR #4031
* PR #4085
* PR #3635
* PR #2356
* PR #2559
* No GH handle - Encore author addition @tbengtsen
* PR #4184
* PR #2614
* PR #2219
* PR #2384
* PR #2668
* Add missing entry for Jenna

Thanks to @fiona-naughton for helping out with digging into this data :)

Co-authored-by: Fiona Naughton <[email protected]>
Co-authored-by: Oliver Beckstein <[email protected]>
@orbeckst
Copy link
Member

I temporarily disabled "PR-required" and enabled "allow force push for coredevs" to make this one amendment to the message of 809ade8. This worked. I reverted the develop branch protection to original settings.

@orbeckst orbeckst deleted the fix-changelog branch December 21, 2023 00:12
@IAlibay
Copy link
Member Author

IAlibay commented Dec 21, 2023

Thanks @orbeckst !

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