search

Automattic / gridicons

The WordPress.com icon set
http://automattic.github.io/gridicons/
GNU General Public License v2.0
110 stars 13 forks source link

New icon: play #232

Closed davewhitley closed 7 years ago

davewhitley commented 7 years ago

screen shot 2017-06-20 at 2 50 20 pm screen shot 2017-06-20 at 2 50 07 pm

fixes #229

folletto commented 7 years ago

For reference, this is pause:

screen shot 2017-06-20 at 21 54 00

I think it works well. 👍

folletto commented 7 years ago

Aside: should we start doing code-review of the SVG during review given the new process?

In this case I'd clean <g id="Artwork">, the group doesn't seem required.

davewhitley commented 7 years ago

Probably. So:

?

folletto commented 7 years ago

I'm slightly for still running grunt just before merge and not for the PR (so it's super simple to contribute for anyone without a grunt setup) — apart from that, yes.

We just check the SVG source code too during the review basically. Nothing else changes (i.e. we expect the SVG added to be a nice and "clean" source ready for running in grunt).

davewhitley commented 7 years ago

Removed , should be good to go now 👍

folletto commented 7 years ago

Checked 🔍
Looks good 👀
Ship it 🚢

SergioEstevao commented 7 years ago

Anyone solving the conflict and merging this soon? :)

davewhitley commented 7 years ago

Yes

davewhitley commented 7 years ago

I messed up the rebase. Changes from other PRs are showing up in this one. Should I make a new fresh branch?

SergioEstevao commented 7 years ago

It will be better to keep it clean from non-related changes.

davewhitley commented 7 years ago

I wasn't able to get this PR in working order. I just created a new branch. #233