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

Added styles for generic shop=* icon #117

Closed
wants to merge 4 commits into from
Closed

Added styles for generic shop=* icon #117

wants to merge 4 commits into from

Conversation

dkniffin
Copy link
Contributor

Resolves Issue #116

Screenshot:
shop-render

@jremillard
Copy link

Could you do this for office, and craft too? Just a dot with correct color?

@dkniffin
Copy link
Contributor Author

yea. so office=* would be blue? and craft=* would be...?

@jremillard
Copy link

If we are not rendering any craft POI, then just a blue office dot would be 110% OK with me. The more focused the patch probably the more likely it will be accepted. They might want the office change to be a new patch.

@dkniffin
Copy link
Contributor Author

Yea, I'd submit it as a separate patch anyhow. It looks like we're not rendering any office POI either. Do you have an example of where one is being rendered?

@dkniffin dkniffin closed this Aug 14, 2013
@dkniffin dkniffin reopened this Aug 14, 2013
@dkniffin
Copy link
Contributor Author

Oops. Sorry for the close/reopen

@jremillard
Copy link

Oh, if office is not being rendered at all, then like the craft tag, I guess should be a separate discussion... I just know that I often have office tags in my POI mapping....

@gravitystorm
Copy link
Owner

See also #116 (comment)

Some technical issues with the pull request are

  • Unnecessary whitespace changes
  • extent_cache and _basemap attributes in project.mml should be kept out of the pull request. I know tilemill can stick them in there automatically, and that's annoying, but use git add -p to avoid including them in the PR

@yohanboniface
Copy link

BTW, an option to manage the project.mml file in TileMill collaborative workflow is to use https://github.com/yohanboniface/CartoCC

@dkniffin
Copy link
Contributor Author

I've made the changes to fix what you said. How do I contribute them, since you closed the pull request?

@matthijsmelissen
Copy link
Collaborator

@oddityoverseer13 I'm sorry that nobody answered your question. If you are still around: you can just create a new pull request. Alternatively, you can push it to this branch and ask one of the people with Collaborator-status to reopen the pull request.

@dkniffin
Copy link
Contributor Author

Oh, I forgot about this. Lemme see if I can find where I put those changes, and I'll push it to this branch.

@dkniffin
Copy link
Contributor Author

Hmm...Still not sure how to do this. My commit is here: https://github.com/oddityoverseer13/openstreetmap-carto/commit/fac836fcef29f78f1bf3ee526434def1010067a4

I'm not sure how to add that to this pull request though. Does it have to be open first?

@matthijsmelissen
Copy link
Collaborator

Strange, it seems indeed that the list of changes does not get updated as long as the pull request is closed. I reopened it now, and the changes show now.

However, the pull request got outdated in the meanwhile: someone else edited the same lines as you did. Would you be able to rebase it on the current version of the code?

@pnorman
Copy link
Collaborator

pnorman commented May 30, 2014

👍 in principle to this.

@dkniffin
Copy link
Contributor Author

Yes, I will make those changes. Probably tomorrow

@dkniffin
Copy link
Contributor Author

I made some changes to my local copy. I still need to test them, but they should be good.

@dkniffin
Copy link
Contributor Author

dkniffin commented Jun 3, 2014

I think I will create a new pull request for this. I tried to re-fork the repo, but I'm seeing a weird issue that might be a bug with github. I emailed support. I'l update this page when I have any relevant information.

@dkniffin
Copy link
Contributor Author

dkniffin commented Jun 4, 2014

I created a new pull request #604. I'm closing this one

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

Successfully merging this pull request may close these issues.

None yet

6 participants