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

docs(readme.md): add repository badges #161

Merged
merged 3 commits into from
Jan 20, 2022
Merged

docs(readme.md): add repository badges #161

merged 3 commits into from
Jan 20, 2022

Conversation

fzn0x
Copy link
Contributor

@fzn0x fzn0x commented Jan 15, 2022

  • provides short information in badges
  • clickable badge to opencollective

provides short information in badges
clickable badge to opencollective
@netlify
Copy link

netlify bot commented Jan 15, 2022

✔️ Deploy Preview for vigilant-wescoff-04e480 ready!

🔨 Explore the source changes: 6e9b019

🔍 Inspect the deploy log: https://app.netlify.com/sites/vigilant-wescoff-04e480/deploys/61e8c9a83dc1670008fed8c8

😎 Browse the preview: https://deploy-preview-161--vigilant-wescoff-04e480.netlify.app

Shinigami92
Shinigami92 previously approved these changes Jan 15, 2022
Copy link
Member

@Shinigami92 Shinigami92 left a comment

Choose a reason for hiding this comment

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

NICE

@Shinigami92 Shinigami92 added c: docs Improvements or additions to documentation c: chore PR that doesn't affect the runtime behavior labels Jan 15, 2022
@Shinigami92 Shinigami92 requested a review from a team January 15, 2022 21:09
ejcheng
ejcheng previously approved these changes Jan 15, 2022
prisis
prisis previously approved these changes Jan 15, 2022
@Shinigami92
Copy link
Member

@damienwebdev you are missing that this will also rendered on npm package site!
So I would use the contributors and license badge.
Also the PR badge shows a healthy activity, at least for now.
We could remove the PR counter when we reached 500+ PRs 🙂

@prisis prisis added this to the v6.0.0 milestone Jan 16, 2022
@fzn0x fzn0x dismissed stale reviews from prisis, ejcheng, and Shinigami92 via a1a9c9e January 19, 2022 05:27
@fzn0x
Copy link
Contributor Author

fzn0x commented Jan 19, 2022

@damienwebdev you are missing that this will also rendered on npm package site! So I would use the contributors and license badge. Also the PR badge shows a healthy activity, at least for now. We could remove the PR counter when we reached 500+ PRs 🙂

Which one I should follow? your suggestion or @damienwebdev? or currently we stale it first?

@prisis
Copy link
Contributor

prisis commented Jan 19, 2022

For me only Merged PRs and contributors can be removed, license should be there for npm

@Shinigami92
Copy link
Member

For me only Merged PRs and contributors can be removed, license should be there for npm

Maybe we should just link it to the license file
Many repos do it that way

@fzn0x
Copy link
Contributor Author

fzn0x commented Jan 19, 2022

Do we follow major consensus vote to remove merged PRs and contributors?

@damienwebdev
Copy link
Member

Do we follow major consensus vote to remove merged PRs and contributors?

We decided to move this change to v6.1.0, but I still want to outline a conversation that @Shinigami92 and I had in Discord:

  1. We don't need the LICENSE badge, we have it built-in on both NPM and Github.
  2. PR Merge count isn't a great ref to project stability, so it should be removed.
  3. Contributors should be removed.

The rest seem good.

@fzn0x
Copy link
Contributor Author

fzn0x commented Jan 20, 2022

Do we follow major consensus vote to remove merged PRs and contributors?

We decided to move this change to v6.1.0, but I still want to outline a conversation that @Shinigami92 and I had in Discord:

  1. We don't need the LICENSE badge, we have it built-in on both NPM and Github.
  2. PR Merge count isn't a great ref to project stability, so it should be removed.
  3. Contributors should be removed.

The rest seem good.

Cool, resolving, thanks @damienwebdev and @Shinigami92 😃

Copy link
Member

@damienwebdev damienwebdev left a comment

Choose a reason for hiding this comment

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

Lgtm

@Shinigami92 Shinigami92 merged commit fa2984b into faker-js:main Jan 20, 2022
@fzn0x fzn0x deleted the patch-2 branch January 20, 2022 13:38
pkuczynski pushed a commit to pkuczynski/faker that referenced this pull request Jan 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c: chore PR that doesn't affect the runtime behavior c: docs Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants