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

Changed my answer to the amd question #32

Merged
merged 5 commits into from
Jul 25, 2013
Merged

Conversation

calvinmetcalf
Copy link
Member

re #20 I changed my answer

got it working too, though ugly as sin and probably would need some help by someone who uses amd modules.

@sheppard
Copy link
Contributor

sheppard commented Jul 3, 2013

Cool, I'll take a look later this week or early next.

@calvinmetcalf
Copy link
Member Author

so since I had to rewrite the test anyway I'm now going through and making sure each projection type is hit with a test

@calvinmetcalf calvinmetcalf mentioned this pull request Jul 8, 2013
@calvinmetcalf
Copy link
Member Author

ok you can now specify a custom build e.g. grunt build --defs=EPSG:2673

@calvinmetcalf
Copy link
Member Author

you can also separate multiple ones with a comma

@calvinmetcalf
Copy link
Member Author

@ahocevar this is mergable from a technical perspective, we may want to discus if it's the right direction though

@ahocevar
Copy link
Member

ahocevar commented Jul 9, 2013

Thanks @calvinmetcalf for your work on this. I'll take a closer look as soon as I get to it.

@@ -0,0 +1,405 @@
/**
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't it be better to use the npm package?

@calvinmetcalf
Copy link
Member Author

yes that would make more sense, fixed

@ahocevar
Copy link
Member

I feel like this is good to merge now, but the pull request seems to include many unrelated commits. Maybe a rebase issue?

@calvinmetcalf
Copy link
Member Author

ok I'll try

@ahocevar
Copy link
Member

If you can update the pull request description to give a summary of all changes, we can also keep all commits in this pull request. Whatever is easier.

@calvinmetcalf
Copy link
Member Author

rebased it with more descriptive commit messeges

@ahocevar
Copy link
Member

Thanks for your continued effort on this! I think this is in great shape now.

ahocevar added a commit that referenced this pull request Jul 25, 2013
Changed my answer to the amd question
@ahocevar ahocevar merged commit 45474c3 into proj4js:master Jul 25, 2013
@ahocevar
Copy link
Member

@calvinmetcalf What should we do with src/projections.js and src/defs/local.js? Should these be moved to dist/ or added to .gitignore?

@calvinmetcalf
Copy link
Member Author

added to gitignore

@calvinmetcalf calvinmetcalf deleted the rjs branch July 25, 2013 15:46
@ahocevar
Copy link
Member

Done with 8c5275a.

@ahocevar ahocevar mentioned this pull request Jul 25, 2013
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.

None yet

3 participants