Skip to content

Conversation

@sahitpj
Copy link

@sahitpj sahitpj commented Aug 2, 2018

This pull request is for adding the Pakistan Map along with it's constituencies

@nthitz
Copy link
Member

nthitz commented Aug 2, 2018

Hi and thanks for your contribution!

Before merging I had a few requests/questions:

  1. Could you rebase onto the latest branch of master and recommit. It looks like your commits remove several of our existing maps.
  2. What's the difference between Pakistan, Pakistan - New & Testing? Probably remove testing, and unless there is a good reason to have both Pakistan & Pakistan - New, can we just stick with one?
  3. Rather than adding that mockIdField function, it'd be better to just update the topojson files with the proper ids.

If you can address these items I'd be happy to get this merged in

@sahitpj
Copy link
Author

sahitpj commented Aug 3, 2018

Yeah I figured, will work on those changes right away

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.

2 participants