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

Add icons for shop=fabric and carpet #3555

Merged
merged 3 commits into from
Dec 16, 2018
Merged

Add icons for shop=fabric and carpet #3555

merged 3 commits into from
Dec 16, 2018

Conversation

Adamant36
Copy link
Contributor

@Tomasz-W Tomasz-W mentioned this pull request Dec 9, 2018
26 tasks
@kocio-pl
Copy link
Collaborator

kocio-pl commented Dec 9, 2018

It seems that shop=sewing has a low usage (<1k uses), so I think you should exclude it from this PR. Especially because it has competitor and it is not clear how it will end up:

taghistory 33

@Adamant36
Copy link
Contributor Author

I think it was disscussed and there was a reason people went with it over haberdashery, but I cant remember the details. I agree the numbers are low either way. So I'll remove it.

@Tomasz-W, do you remember anything about it?

@Tomasz-W
Copy link

@Adamant36 No, it was a quite long and complitated discussion. But as these numbers are small, we can left this icon and add it in the future when situation will be clear.

@Tomasz-W
Copy link

@Adamant36 Can you test v2 of 'roll + pattern'? I've mirrored pattern part, I think it looks better in this way.

https://gist.github.com/Tomasz-W/24baa83bbed6e1b56b6d86c54d56579f

@kocio-pl
Copy link
Collaborator

@Adamant36 Do you plan to test it?

@Adamant36
Copy link
Contributor Author

Yeah sorry. I've been hella busy lately. I was just thinking about it though. So I'll do it sometime this weekend. Thanks for reminding me.

@kocio-pl
Copy link
Collaborator

Great! Please also remember to remove sewing section.

@Adamant36
Copy link
Contributor Author

I will.

@Adamant36
Copy link
Contributor Author

Fabric roll + pattern reversed. I don't really have a preference. So I'm good with either.
fabric reversed

@kocio-pl kocio-pl changed the title Add icons for shop=fabric, carpet, sewing Add icons for shop=fabric and carpet Dec 16, 2018
@kocio-pl
Copy link
Collaborator

I also don't see the difference, so no problem... Thanks for you both!

@Adamant36
Copy link
Contributor Author

@matkoniecz, were you able to figure the problem out? It ran fine when I tested it.

@matkoniecz
Copy link
Contributor

Yes, as mentioned it was insufficient checking on my side.

@Adamant36
Copy link
Contributor Author

Thats good. I just thought id double check just in case.

@kocio-pl
Copy link
Collaborator

I'm always testing patches before merging, so at least they should really work and code should compile (but there can be other problems, of course).

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

Successfully merging this pull request may close these issues.

Add special icon for shop=fabric
4 participants