-
Notifications
You must be signed in to change notification settings - Fork 107
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
Remove CasperJS from the GeoExt repository only installing it via NPM if needed. #285
Conversation
I like this. I always wanted to remove the copied casperjs, but I am glad you finally did it. This change does introduce I'd love to hear a short sentence from @chrismayer whether everything works for him on his windows machine, but otherwise this looks great to me. Thanks anyhow, I am sure that we'll merge this sooner or later. Hey @bentrm, you may be interested to also have a look at #286 while you're looking at headless testing. |
Good point, I kind of disregarded Windows. Though, I'm not sure if the headless tests have been working on Windows anyway. I just tried it with a fresh clone on a development machine. Seems like you have to run |
I am pretty sure they didn't really work on windows. Let's see if @chrismayer finds out something. npm should help at least with the installation, right? I personally don't think this should block this, as the headless tests are an added value for CI mainly. And I really want to remove all the doubled content (14k lines). |
No, it did not work on Windows before. So we do not break anything with this PR and I am +1 for merging this. Thanks for tackling this! |
Thanks @chrismayer & @bentrm. |
Remove CasperJS from the GeoExt repository only installing it via NPM if needed.
Awesome 😏 👍 |
These changes aimed to remove unrelated files from the GeoExt repository by removing the duplicated CasperJS repository. Readme.md, .travis.yml and .gitignore have been updated accordingly to make sure everything still works as expected.
I hope I'm not intruding on other build mechanics that are not apparent via the repo. If these changes are a no-go, I'm ok with it.