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

Update contributors #2143

Merged
merged 11 commits into from Aug 28, 2021
Merged

Update contributors #2143

merged 11 commits into from Aug 28, 2021

Conversation

casperklein
Copy link
Member

Description

This combines two approaches to list all contributors.

  1. All repository contributors are automatically added / updated with a daily workflow
  2. Custom contributors can be added via the @all-contributors bot

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Improvement (non-breaking change that does improve existing functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation (README.md or the documentation under docs/)
  • If necessary I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

@casperklein casperklein requested a review from a team August 22, 2021 16:19
@casperklein casperklein self-assigned this Aug 22, 2021
Copy link
Member

@georglauterbach georglauterbach left a comment

Choose a reason for hiding this comment

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

LGTM 👍

polarathene
polarathene previously approved these changes Aug 22, 2021
Copy link
Member

@polarathene polarathene left a comment

Choose a reason for hiding this comment

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

This is going to remove the current contribution types? Were those going to be added back or automated in some way? Or are we no longer seeing any value in them?

@casperklein
Copy link
Member Author

I didn't considered this as very important. That's the drawback in favor of the automated listing of all code contributors.

We still can add everyone to the "second list" with the all-contributors bot. So this possibility is still given if we want that.

Copy link
Member

@wernerfred wernerfred left a comment

Choose a reason for hiding this comment

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

Can we somehow make sure that someone that gets added to "further contributors" is not already on the "normal" contributors.

I'm still not sure on this whole topic as the main benefit (showing on which topic someone contributed) gets lost / in the background (bc there are 200+ normal contributors in front).
We could rely on the github contributors list alone and have no effort at all.

CONTRIBUTORS.md Show resolved Hide resolved
CONTRIBUTORS.md Outdated Show resolved Hide resolved
@casperklein
Copy link
Member Author

Can we somehow make sure that someone that gets added to "further contributors" is not already on the "normal" contributors.

I've no easier idea, how to handle this, as by looking first at the contributors before adding someone manually via bot.

We should include an information why this section is separated from the first one.

Good point. Just "fixed" that. If you have a better description/phrasing, let me know and I'll happily change it 👍

I'm still not sure on this whole topic as the main benefit
We could rely on the github contributors list alone and have no effort at all.

I've worked on this PR, because I thought, based on the feedback + "thumbs up" in the maintainer discussion, there is consent (I may be wrong with that). At the end, it's a question of faith:

To have one central place where we:
Honor new contributions broken down to explicit topics VS. just honor all contributions without showing in detail the exact contribution areas.

I am the opinion with this "hybrid" model, it's a good compromise. I think all pro/cons have been mentioned in the maintainer discussion, so everyone can make a decision. To come to an end:

@docker-mailserver/maintainers

If you in general disagree with this approach at all and just want to leave it as it is right now, 👎 this post.
If you in general agree with this approach and like it to be merged, 👍 this post.

@polarathene
Copy link
Member

polarathene commented Aug 23, 2021

I don't mind either way, as @wernerfred mentioned, we have official github repo stats on contributors which looks to be sorted by number of commits from contributors. All of us rank high on that and are visible.

From what I understood, this automates the alternative contributors document that was setup, and still allows for making additions (such as different contribution badges) via the existing bot if desired. Sounds win/win to me 🤷‍♂️


I'm still not sure on this whole topic as the main benefit (showing on which topic someone contributed) gets lost / in the background (bc there are 200+ normal contributors in front).

I think the value is meant to be in stuff like the contribution type badges? This doesn't matter if you get buried, as long as you can easily ctrl+f locate yourself in the document, you have access to any of that data, it's doubtful anyone else is going to look through it for more than one user.

The badges are one of the main differences from what Github officially provides. If we want a list of top contributors, we can use that instead. IIRC that wasn't sufficient for some when creating this contributors doc/section was discussed.

We could rely on the github contributors list alone and have no effort at all.

I don't recall exact reasons for this alternative document, but someone wanted to give contributors more recognition/visibility and introduced this. Then we had an issue about manually managing it and wanting to not leave out past contributors, so automation made sense?

@casperklein
Copy link
Member Author

casperklein commented Aug 28, 2021

@wernerfred Is the current solution for you acceptable?

I want to merge this the next days, if there is no veto from someone.

@wernerfred
Copy link
Member

Do it 🚀

@casperklein casperklein enabled auto-merge (squash) August 28, 2021 13:22
@casperklein casperklein merged commit 6ed4f8e into docker-mailserver:master Aug 28, 2021
@wernerfred wernerfred added this to the v10.1.1 milestone Aug 28, 2021
@casperklein casperklein deleted the contrib branch September 28, 2021 00:49
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.

None yet

4 participants