HatScripts / circle-flags

A collection of 400+ minimal circular SVG country, state and language flags
https://hatscripts.github.io/circle-flags
MIT License
945 stars 249 forks source link

Add contribution instructions #3

Closed waldyrious closed 4 years ago

waldyrious commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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:

waldyrious commented 4 years ago

I will add it to the README in a new PR

Update: this has now been addressed in #4.