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

Updating guidelines unofficial source #6428

Merged
merged 19 commits into from Sep 3, 2021
Merged

Updating guidelines unofficial source #6428

merged 19 commits into from Sep 3, 2021

Conversation

jorgeamadosoria
Copy link
Contributor

Issue: #6418

Adding a textual amendment to contribution guidelines to address de facto standard icons to be added to the library via consensus of maintainers.
The text in the change is merely a proposal to get the ball rolling, I don't mind changing it completely as long as the spirit of it remains.

@jorgeamadosoria jorgeamadosoria added the docs Issues for improving or updating package documentation label Aug 29, 2021
Copy link
Contributor

@ericcornelissen ericcornelissen left a comment

Choose a reason for hiding this comment

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

I think the current approach is not correct.

  • First the bullet list forms a set of conditions, all of which have to be met in order for the icon to be accepted. The bullet you added is an or-condition with the bullet preceding it, which is confusing.
  • This section should probably not cover how the logo should be chosen, none of the other bullets and the title doesn't indicate the section will talk about logos.

I think it makes more sense to add it in the "1. Identify Official Logos and Colors" section.

@ericcornelissen ericcornelissen added in discussion There is an ongoing discussion that should be finished before we can continue meta Issues or pull requests regarding the project or repository itself and removed docs Issues for improving or updating package documentation labels Aug 29, 2021
@sachinraja sachinraja linked an issue Aug 29, 2021 that may be closed by this pull request
@jorgeamadosoria
Copy link
Contributor Author

I think the current approach is not correct.

  • First the bullet list forms a set of conditions, all of which have to be met in order for the icon to be accepted. The bullet you added is an or-condition with the bullet preceding it, which is confusing.
  • This section should probably not cover how the logo should be chosen, none of the other bullets and the title doesn't indicate the section will talk about logos.

I think it makes more sense to add it in the "1. Identify Official Logos and Colors" section.

changed taking this input into account

CONTRIBUTING.md Outdated Show resolved Hide resolved
Co-authored-by: Álvaro Mondéjar <mondejar1994@gmail.com>
CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
jorgeamadosoria and others added 5 commits August 31, 2021 15:44
Co-authored-by: Eric Cornelissen <ericornelissen@gmail.com>
Co-authored-by: Eric Cornelissen <ericornelissen@gmail.com>
Co-authored-by: Sachin Raja <sachinraja2349@gmail.com>
reduced the logo description to "widely accepted" rather than "well known or widely accepted de facto standard"
@jorgeamadosoria
Copy link
Contributor Author

Is this ready for merging now @ericcornelissen @sachinraja ?

CONTRIBUTING.md Outdated Show resolved Hide resolved
mondeja
mondeja previously requested changes Sep 2, 2021
_data/simple-icons.json Outdated Show resolved Hide resolved
@ericcornelissen ericcornelissen removed the in discussion There is an ongoing discussion that should be finished before we can continue label Sep 2, 2021
CONTRIBUTING.md Outdated Show resolved Hide resolved
Co-authored-by: Sachin Raja <sachinraja2349@gmail.com>
@jorgeamadosoria
Copy link
Contributor Author

Thanks @mondeja and @sachinraja for the fixes. What do you think is pending yet?

@jorgeamadosoria jorgeamadosoria merged commit 8e5f40e into simple-icons:develop Sep 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
meta Issues or pull requests regarding the project or repository itself
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Popular icons from unofficial sources
4 participants