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

Add contribution instructions #3

Merged
merged 1 commit into from
Jun 9, 2020

Conversation

waldyrious
Copy link
Collaborator

@waldyrious waldyrious commented Jun 9, 2020

Added content to the README to make it easier to contribute to this repo.

I also made a small adjustment to the existing README content.

@waldyrious
Copy link
Collaborator Author

By the way, @HatScripts: following these steps locally leads to slightly different output when running the optimization script — pretty much all flags are changed, with diffs like this:

-<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 512 512"><path fill="#ffda44" d="M144.7 486.6C178.4 502.9 216.1 512 256 512s77.6-9.1 111.3-25.4L389.6 256 367.3 25.4C333.6 9.1 295.9 0 256 0s-77.6 9.1-111.3 25.4L122.4 256l22.3 230.6z"/><path fill="#d80027" d="M367.3 486.6a256 256 0 0 0 0-461.2v461.2z"/><path fill="#0052b4" d="M144.7 486.6V25.4a256 256 0 0 0 0 461.2z"/><path fill="#d80027" d="M256 345v-89h66.8v33.4c0 5.8-11.1 27-38.6 44.5-10.4 6.6-21.2 8.8-28.2 11.1zm-66.8-155.8H256V256h-66.8z"/><path fill="#ff9811" d="M289.4 167a22.3 22.3 0 0 0-33.4-19.3 22.1 22.1 0 0 0-11.1-3c-12.3 0-22.3 10-22.3 22.3H167v111.3c0 41.4 32.9 65.4 58.7 77.8a22.1 22.1 0 0 0-3 11.2 22.3 22.3 0 0 0 33.3 19.3 22.1 22.1 0 0 0 11.1 3 22.3 22.3 0 0 0 19.2-33.5c25.8-12.4 58.7-36.4 58.7-77.8V167h-55.6zm22.3 111.3c0 5.8 0 23.4-27.5 40.9a136.5 136.5 0 0 1-28.2 13.3c-7-2.4-17.8-6.7-28.2-13.3-27.5-17.5-27.5-35.1-27.5-41v-77.9h111.4v78z"/></svg>
\ No newline at end of file
+<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 512 512"><path fill="#ffda44" d="M144.7 486.6C178.4 502.9 216.1 512 256 512s77.6-9.1 111.3-25.4L389.6 256 367.3 25.4C333.6 9.1 295.9 0 256 0s-77.6 9.1-111.3 25.4L122.4 256l22.3 230.6z"/><path fill="#d80027" d="M367.3 486.6a256 256 0 000-461.2v461.2z"/><path fill="#0052b4" d="M144.7 486.6V25.4a256 256 0 000 461.2z"/><path fill="#d80027" d="M256 345v-89h66.8v33.4c0 5.8-11.1 27-38.6 44.5-10.4 6.6-21.2 8.8-28.2 11.1zm-66.8-155.8H256V256h-66.8z"/><path fill="#ff9811" d="M289.4 167a22.3 22.3 0 00-33.4-19.3 22.1 22.1 0 00-11.1-3c-12.3 0-22.3 10-22.3 22.3H167v111.3c0 41.4 32.9 65.4 58.7 77.8a22.1 22.1 0 00-3 11.2 22.3 22.3 0 0033.3 19.3 22.1 22.1 0 0011.1 3 22.3 22.3 0 0019.2-33.5c25.8-12.4 58.7-36.4 58.7-77.8V167h-55.6zm22.3 111.3c0 5.8 0 23.4-27.5 40.9a136.5 136.5 0 01-28.2 13.3c-7-2.4-17.8-6.7-28.2-13.3-27.5-17.5-27.5-35.1-27.5-41v-77.9h111.4v78z"/></svg>
\ No newline at end of file

Essentially, sequences of multiple zeroes like 0 0 0 foo or 0 0 0 0 bar have the latter zeroes combined, becoming 0 00foo and 0 000bar. This doesn't seem to be causing issues with the rendering, but I wanted to be sure.

I believe it may be due to using a newer version of svgo. The version I have locally is 1.3.2, but at the time of your last commit the last release of svgo was 1.1.1. I can submit a new PR re-optimizing all flags with svgo's newest output, to prevent other contributors from running into such discrepancy.

@waldyrious
Copy link
Collaborator Author

Hmm, actually this PR should probably not be merged as-is; PR svgo#712, mentioned in the comment in compress.py, has since been merged and released with svgo 1.2.0.

So maybe I should simply remove compress.py and change the instructions in this PR to run svgo directly, right?

@waldyrious waldyrious force-pushed the contributing-instructions branch 2 times, most recently from 719573a to 38cbac0 Compare June 9, 2020 16:45
@waldyrious
Copy link
Collaborator Author

I've tested using just svgo with the --recursive flag, and it works :) I've updated the PR to remove the compress.py file and provide simpler instructions in the README.

@waldyrious waldyrious force-pushed the contributing-instructions branch 3 times, most recently from a6ba2b1 to 4bccda4 Compare June 9, 2020 16:49
@waldyrious
Copy link
Collaborator Author

I can submit a new PR re-optimizing all flags with svgo's newest output, to prevent other contributors from running into such discrepancy.

Opened #4 adding these changes.

@HatScripts
Copy link
Owner

HatScripts commented Jun 9, 2020

Love your stuff! 🤟

It's nice to see the python script is no longer necessary, though it did have a lovable hackiness to it. Anyway that should hopefully make this repo more accessible to further contributors. Also I noticed you excluded the --config=svgo.yml when calling svgo in the README. I take it that this is arg not needed when svgo.yml is already in the cwd?

@waldyrious
Copy link
Collaborator Author

waldyrious commented Jun 10, 2020

I noticed you excluded the --config=svgo.yml when calling svgo in the README. I take it that this is arg not needed when svgo.yml is already in the cwd?

Actually that wasn't intentional! I just tested and it needs to be passed explicitly to the command. I will add it to the README in a new PR, but just for curiosity, why did you set so many options in svgo.yml that are already the default in svgo? Specifically, these:

  • removeTitle: true
  • removeDesc: true
  • removeUselessDefs: true
  • removeHiddenElems: true
  • removeUnknownsAndDefaults: true
  • removeUselessStrokeAndFill: true
  • removeUnusedNS: true
  • cleanupIDs: true
  • cleanupNumericValues: true
  • collapseGroups: true

@waldyrious waldyrious deleted the contributing-instructions branch June 10, 2020 00:25
@waldyrious
Copy link
Collaborator Author

I will add it to the README in a new PR

Update: this has now been addressed in #4.

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.

2 participants