Logos cleanup and AppVeyor #812

techtonik · 2016-10-19T18:37:17Z

This adds AppVeyor logo to its badge. Fixes #507.

techtonik · 2017-04-25T09:00:18Z

Rebased and squashed.

paulmelnikow · 2017-04-25T15:50:34Z

lib/load-logos.js

  logoFiles.forEach(function(filename) {
    if (filename[0] === '.') { return; }
    // filename is eg, github.svg
-    var svg = fs.readFileSync(
-      path.join(__dirname, '..', 'logo', filename)).toString();
+    var svg = fs.readFileSync(logoDir + '/' + filename).toString();


Can you use a call to path.join here? It works better across platforms.

On my Windows it works okay, so I don't see why it is needed here. It probably matters if you have to call some external Windows program and pass the path there, which is not the case.

It looks like the node internal functions handle either okay.

Windows internally supports forward slashes. The main purpose of path.join() in practice is path normalization, eg. path.join('foo/', '..', 'bar') === 'bar'.

paulmelnikow · 2017-04-25T15:51:36Z

logo/appveyor.svg

-<svg xmlns="http://www.w3.org/2000/svg" width="40" height="40" viewBox="0 0 40 40">
-<path fill="#09A9ED" d="M20 0c11 0 20 9 20 20s-9 20-20 20S0 31 0 20 9 0 20 0zm4.9 23.9c2.2-2.8 1.9-6.8-.9-8.9-2.7-2.1-6.7-1.6-9 1.2-2.2 2.8-1.9 6.8.9 8.9 2.8 2.1 6.8 1.6 9-1.2zm-10.7 13c1.2.5 3.8 1 5.1 1L28 25.3c2.8-4.2 2.1-9.9-1.8-13-3.5-2.8-8.4-2.7-11.9 0L2.2 21.6c.3 3.2 1.2 4.8 1.2 4.9l6.9-7.5c-.5 3.3.7 6.7 3.5 8.8 2.4 1.9 5.3 2.4 8.1 1.8l-7.7 7.3z"/>
+<svg xmlns='http://www.w3.org/2000/svg' width='40' height='40' viewBox='0 0 40 40'>
+<path fill='#ddd' d='M20 0c11 0 20 9 20 20s-9 20-20 20S0 31 0 20 9 0 20 0zm4.9 23.9c2.2-2.8 1.9-6.8-.9-8.9-2.7-2.1-6.7-1.6-9 1.2-2.2 2.8-1.9 6.8.9 8.9 2.8 2.1 6.8 1.6 9-1.2zm-10.7 13c1.2.5 3.8 1 5.1 1L28 25.3c2.8-4.2 2.1-9.9-1.8-13-3.5-2.8-8.4-2.7-11.9 0L2.2 21.6c.3 3.2 1.2 4.8 1.2 4.9l6.9-7.5c-.5 3.3.7 6.7 3.5 8.8 2.4 1.9 5.3 2.4 8.1 1.8l-7.7 7.3z'/>


Why change the quote style from " to '?

Saves size - https://codepen.io/tigt/post/optimizing-svgs-in-data-uris

Interesting. Unfortunately changing the quote style makes no size reduction Shields, since the logos are base64-encoded, not URL-encoded.

Let's do this in a concerted way. Could you leave/use double quotes for now, and open an issue or a separate PR to drop base64 encoding and change the quote style?

Possibly we could change the quote style on the fly, as that post suggests, though modifying the checked-in logos could makes sense too. I'd like to benchmark the response size changes, to make sure the optimization is having the desired effect, and also do some browser testing.

I can commit quotes for other logos in a separate commit for testing.

Could you reset them here as well?

I'd like to avoid spending more time on this. There are design fixes inside. I wish I submitted the logo with single quotes from the start,

paulmelnikow · 2017-04-25T15:52:22Z

logo/bithound.svg

Could you reset these files? They seem to be unrelated to this change.

paulmelnikow · 2017-04-25T15:53:56Z

Hi, thanks for this and for rebasing. Would like to get this merged.

Just a testing code: var loadLogos = require('./lib/load-logos.js'); var logos = loadLogos(); console.log(logos);

Fixes badges#507 Double quotes help to save bytes when SVGs are urlencoded. See https://codepen.io/tigt/post/optimizing-svgs-in-data-uris

techtonik · 2017-04-26T07:19:05Z

@paulmelnikow left logo size optimization out of commit for now.

techtonik · 2017-04-28T09:07:07Z

@paulmelnikow thanks for taking care of this.

billziss-gh · 2017-05-01T19:29:21Z

For what it's worth I personally dislike this change.

One of the reasons that I use shields.io badges it that I can customize them. Having a logo added to what was a "custom" badge is a breaking change from my point of view.

I use Travis CI for OSX/Linux builds and AppVeyor for Windows builds. To differentiate between the builds I have custom shields.io badges that read "osx/linux " for Travis CI and "windows " for appveyor.

From cgofuse:

Please consider not adding the logo on "custom" badges (i.e. ones with labels, etc). Alternatively add logo images for all services (e.g. Travis CI).

techtonik · 2017-05-01T20:15:30Z

@billziss-gh if you can find optimized logo for Travis CI in SVG along with its license, that can help it.

billziss-gh · 2017-05-01T21:00:51Z

@techtonik thanks for the quick answer and opening up a new issue on this.

paulmelnikow · 2017-05-01T22:54:23Z

Hi @billziss-gh, thanks for the feedback + for opening an issue @techtonik.

I like supporting these logos on flat badges. We could consider making them opt in. The principle should be to do what's right for badge consumers as a whole, in the long run.

I posted a comment in #983. Let's continue there.

techtonik changed the title ~~Move load-logos.js into lib/~~ Logos cleanup and AppVeyor Oct 19, 2016

techtonik referenced this pull request in techtonik/shields Jan 23, 2017

Add logo to AppVeyor badge and change logo color

e6e4832

Fixes badges#507

marcelstoer mentioned this pull request Jan 25, 2017

added appveyor tests status badge #817

Closed

paulmelnikow added the enhancement label Mar 27, 2017

paulmelnikow added frontend The Docusaurus app serving the docs site service-badge New or updated service badge and removed frontend The Docusaurus app serving the docs site labels Apr 13, 2017

techtonik force-pushed the logos branch from a381cbc to 388209b Compare April 25, 2017 08:56

paulmelnikow reviewed Apr 25, 2017

View reviewed changes

techtonik added 2 commits April 26, 2017 08:22

Document and simplify loadLogos

d1ec265

Just a testing code: var loadLogos = require('./lib/load-logos.js'); var logos = loadLogos(); console.log(logos);

Add logo to AppVeyor badge and change logo color

9877c23

Fixes badges#507 Double quotes help to save bytes when SVGs are urlencoded. See https://codepen.io/tigt/post/optimizing-svgs-in-data-uris

techtonik force-pushed the logos branch from 388209b to 9877c23 Compare April 26, 2017 07:16

Use double quotes like other logos

3954ffb

paulmelnikow merged commit 6c34191 into badges:master Apr 28, 2017

paulmelnikow added this to the Next Deploy milestone Apr 28, 2017

techtonik deleted the logos branch April 28, 2017 09:08

techtonik mentioned this pull request May 1, 2017

Support logo= to turn off the badge #983

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Logos cleanup and AppVeyor #812

Logos cleanup and AppVeyor #812

techtonik commented Oct 19, 2016 •

edited

Loading

techtonik commented Apr 25, 2017

paulmelnikow Apr 25, 2017

techtonik Apr 26, 2017

paulmelnikow Apr 26, 2017

espadrine Apr 28, 2017 •

edited

Loading

paulmelnikow Apr 25, 2017

techtonik Apr 26, 2017

paulmelnikow Apr 26, 2017

techtonik Apr 26, 2017

paulmelnikow Apr 26, 2017

techtonik Apr 26, 2017

paulmelnikow Apr 25, 2017

paulmelnikow commented Apr 25, 2017

techtonik commented Apr 26, 2017

techtonik commented Apr 28, 2017 •

edited

Loading

billziss-gh commented May 1, 2017 •

edited

Loading

techtonik commented May 1, 2017

billziss-gh commented May 1, 2017

paulmelnikow commented May 1, 2017

Logos cleanup and AppVeyor #812

Logos cleanup and AppVeyor #812

Conversation

techtonik commented Oct 19, 2016 • edited Loading

techtonik commented Apr 25, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

espadrine Apr 28, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

paulmelnikow commented Apr 25, 2017

techtonik commented Apr 26, 2017

techtonik commented Apr 28, 2017 • edited Loading

billziss-gh commented May 1, 2017 • edited Loading

techtonik commented May 1, 2017

billziss-gh commented May 1, 2017

paulmelnikow commented May 1, 2017

techtonik commented Oct 19, 2016 •

edited

Loading

espadrine Apr 28, 2017 •

edited

Loading

techtonik commented Apr 28, 2017 •

edited

Loading

billziss-gh commented May 1, 2017 •

edited

Loading