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

Rename some SVG identifers for clarity #39

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Ericson2314
Copy link
Member

@samueldr OK I understand the layers in the snowflake better now. Sorry for the deluge of PRs, this one should be the safest and most conservative place to start. I'll rebase / reevaluate other things in order or increasing severity.

@samueldr
Copy link
Member

samueldr commented Oct 8, 2018

No worries! And thanks again for taking time for this! These are all things I have gotten familiar with by playing around with stuff in here.

This PR is the right kind of improvement all around! While it won't matter much for editing with Inkscape, it will make things easier when doing things with the source.

Though, might I suggest naming them lambda-with-gradient-[dark|light]-X instead of lambda-gradient-[dark|light]-X; the gradients themselves have IDs, I initially thought that the lambda-gradient-... where the gradient references and not the lambdas themselves. What do you think of this?

Though I now see that in one file, one gradient identifier was changed, they weren't all changed, and not touched in other files. Was this done through grep/sed through the files for the known identifiers? Is there more work waiting for this PR?

(Yay bikeshedding where no one will see!)

@Ericson2314
Copy link
Member Author

Hmm yeah everything is did there was repo wide sed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants