JetBrains / logos

Other
27 stars 9 forks source link

Clean up SVG for web via SVGCleaner #11

Closed eugene-lazarev closed 4 years ago

eugene-lazarev commented 4 years ago

Hi there! I just cleaned all your SVG in /web directory using SVGCleaner (desktop version).

Снимок экрана 2020-05-22 в 17 36 02

Please make sure you have cleaned your SVG on committing next time.

Don’t forget to push all of these beautiful clean SVG on your current web-based products.

Thank you!

andrey-skl commented 4 years ago

@eugene-lazarev Thank you for your involvement!

Unlikely we prefer another workflow for cleaning up SVGs: we store raw sources in repository and minimize it with SVGO on CI server before publishing. So, NPM package contains minimized and cleaned versions. You can check it by installing NPM package (npm install @jetbrains/logos) or by using unpkg.com. We use that minified versions in all our web products.

This is similar to what web developers usually do with other source code: they store sources in the repository and minimize on build. This works pretty well for us.

Do you observe any issues with our minified files? I hope not, but if yes, please report exact issues with exact files. We may fine-tune our SVGO config if needed.

Thank you again

eugene-lazarev commented 4 years ago

I’m sorry, @andrey-skl, but yes, I do observe an issue right here on your YouTrack project: https://youtrack.jetbrains.com/_classpath/smartui/img/default/youtrack-sign.svg

The production’s SVG is not-minified.

I believe there is NO reason to store dirty SVG here in Github. Cleaned SVG are the same, but smaller.

Please re-open and accept it.

Thank you.

andrey-skl commented 4 years ago

You right, this logotype was stored in YouTrack sources and was not minified. It has no connection with @jetbrains/logos package, so this pull request won't fix it.

Likely it has been just fixed a few days ago, see https://youtrack.jetbrains.com/issue/JT-57522, so this is not an issue anymore.

As for the way we store SVGs in GitHub – it matters for us to store non-minified sources.

I understand you don't like this way, but this is our way to work with logos and we are pretty happy with it. If you want to have your own workflow, you are free to fork this repo and use it in any way you want.

I hope you understand, Best regards

eugene-lazarev commented 4 years ago

Thank you for the clarification.

Likely it has been just fixed a few days ago, see https://youtrack.jetbrains.com/issue/JT-57522, so this is not an issue anymore.

I don’t have access to this issue.

  • This allows us to have a proper diff and history when designers change something in the logotype.

Do you really need a history of changing code row-by-row in SVG? Why? How do you use this? SVG is the whole file, unlike JS or any other code. Designers are using Adobe Illustrator (thanks for this information, dirty SVG) and they don’t change SVG-code itself.

  • This simplifies working with the sources because, if every commiter has to use minification before commit, it adds additional tension and is error-prone.

Yes, there are some code-style rules for commits, and this one could be just another one. Also you can use pre-commit hooks. As the compromise, you could create dist directory and put cleaned SVG there, like many other developers here on Github.

I understand that you love the way you use it, and I see you prefer to keep it. Sorry to waste your time.

Thanks.

andrey-skl commented 4 years ago

I don’t have access to this issue.

Sorry, we've change visibility and now it is visible. Actually it was reported and fixed after your twitter feedback, so thank you for it!