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

Clarify GitHub All Contributors badge #7690

Merged
merged 3 commits into from
Mar 6, 2022

Conversation

PyvesB
Copy link
Member

@PyvesB PyvesB commented Mar 6, 2022

I was wondering what the difference was between these two badges and had to go digging through the code to figure things out:
Screenshot 2022-03-06 121939

Changing the title slightly should help clarify this.

@PyvesB PyvesB added the service-badge Accepted and actionable changes, features, and bugs label Mar 6, 2022
@shields-ci
Copy link

shields-ci commented Mar 6, 2022

Warnings
⚠️ This PR modified service code for github but not its test code.
That's okay so long as it's refactoring existing code.
Messages
📖 ✨ Thanks for your contribution to Shields, @PyvesB!

Generated by 🚫 dangerJS against bfc910c

calebcartwright
calebcartwright previously approved these changes Mar 6, 2022
Copy link
Member

@calebcartwright calebcartwright left a comment

Choose a reason for hiding this comment

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

This one has tripped me up on multiple occasions too so agreed a change is needed. I wonder if it might also be helpful to extend the help text to provide more context about All Contributors too

@PyvesB
Copy link
Member Author

PyvesB commented Mar 6, 2022

extend the help text to provide more context about All Contributors too

Hmm, I'm not sure. What specifically did you have in mind? I see the UI documentation as a place where we surface caveats or what steps need to be taken to get the badge working. Adding context about individual services feels out of scope, the allcontributors.org will probably do a better job than any description we come up with. In that sense, I'd rather fully delegate to it by just surfacing the hostname in the title.

@calebcartwright
Copy link
Member

calebcartwright commented Mar 6, 2022

To be clear, I'm not suggesting help text as a mutually exclusive alternative to the example title.

I think an extra one liner in the modal window help text that includes a hyperlink to the service and mentions that All Contributors defines/counts contributors very differently than GitHub is useful context to a casual observer. Yes, folks can also go and read the details (which is where i think a navigable hyperlink is helpful), but IMO it's also useful to give users a quick inline blurb within the context of Shields to convey they are actually different metrics (which is why they are different badges). From my perspective that hits the definition of a caveat given how identical the two look/sound at first blush.

My prior approval was intended as an implicit 👍 on this as-is; consider the idea about help text as a "yes, and" notion that's not a blocking point for me whatsoever, so definitely feel free to go ahead and merge especially if you'd still prefer to not modify the help text

@shields-cd shields-cd temporarily deployed to shields-staging-pr-7690 March 6, 2022 17:50 Inactive
@PyvesB PyvesB temporarily deployed to shields-staging-pr-7690 March 6, 2022 18:32 Inactive
@PyvesB
Copy link
Member Author

PyvesB commented Mar 6, 2022

I've given it a go, let me know what you think.

Copy link
Member

@calebcartwright calebcartwright left a comment

Choose a reason for hiding this comment

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

lgtm!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
service-badge Accepted and actionable changes, features, and bugs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants