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

mapnik3 #38

Merged
merged 11 commits into from
Oct 3, 2014
Merged

mapnik3 #38

merged 11 commits into from
Oct 3, 2014

Conversation

yhahn
Copy link
Member

@yhahn yhahn commented Oct 3, 2014

This will require a major semver version bump -- (2.0.0)

@yhahn
Copy link
Member Author

yhahn commented Oct 3, 2014

@yhahn yhahn changed the title Mapnik3 [notready] mapnik3 Oct 3, 2014
@springmeyer
Copy link
Contributor

Interesting. Those Mapnik LOG entries are warnings rather than errors and should be happening I think. So, I'm confused then why mocha is actually erroring...

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling fcb97d2 on mapnik3 into 6f5cde0 on master.

@springmeyer
Copy link
Contributor

So, I'm realizing that mapnik-omnivore has never worked on appveyor. All previous runs were against node v0.11.x which failed to ever run the tests because node-gdal fails to build against node v0.11.x. Due to the allow_failures all builds looked like they were passing.

Now that we are testing against node v0.10.x the tests are actually running.

I see that the mapnik3 branch tests work just fine on my Windows VM so the problem is not mapnik-omnivore but rather that appveyor+node v0.10+mocha is borking on the STDERR from Mapnik's logging. The logging I feel is important because layers without features are odd. So we can just ignore these test failures, port to tap, or move on mapnik/node-mapnik#300 to be able to silent the logging.

@GretaCB
Copy link
Contributor

GretaCB commented Oct 3, 2014

Yeah, the warning logs are expected for GPX sources. It's happening somewhere in here: https://github.com/mapbox/mapnik-omnivore/blob/master/lib/datasourceProcessor.js#L613-L623
A little while back, Dane and I chatted about this, and I just decided to leave the warning logs.

Perfect (d6fb44b), thanks @yhahn . I just did the same in unpacker tests for center. Was wondering if this would pop up in other local env's in mapnik-omnivore.

@springmeyer , we ended up having to change the mocha script (9baff42) a while back due to something like this. I'm assuming this is related?

@springmeyer
Copy link
Contributor

@springmeyer , we ended up having to change the mocha script (9baff42) a while back due to something like this. I'm assuming this is related?

Hmm, can you say more? I don't know.

@GretaCB
Copy link
Contributor

GretaCB commented Oct 3, 2014

@springmeyer The ticket I was thinking of (https://github.com/mapbox/DEAD-unpacker-shapefile/issues/14). This was in unpacker before mapnik-omnivore was split out.

@yhahn
Copy link
Member Author

yhahn commented Oct 3, 2014

: | having trouble tracking the appveyor issue down -- can't reproduce these fails on my VM. I think next action for me is to switch the tests over to tape from mocha which has been much better for us on appveyor.

@GretaCB
Copy link
Contributor

GretaCB commented Oct 3, 2014

@yhahn I can work on switching over to tape

@yhahn
Copy link
Member Author

yhahn commented Oct 3, 2014

Tradin

@yhahn yhahn assigned yhahn and unassigned GretaCB Oct 3, 2014
@yhahn yhahn mentioned this pull request Oct 3, 2014
@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling 2e478c8 on mapnik3 into 6f5cde0 on master.

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling f1eb504 on mapnik3 into * on master*.

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling dd33b39 on mapnik3 into * on master*.

@yhahn yhahn changed the title [notready] mapnik3 mapnik3 Oct 3, 2014
@yhahn
Copy link
Member Author

yhahn commented Oct 3, 2014

👍 tape 2/3 times on appveyor and time #2 actually was a pass but appveyor didnt know it

yhahn added a commit that referenced this pull request Oct 3, 2014
@yhahn yhahn merged commit 323b926 into master Oct 3, 2014
@yhahn yhahn deleted the mapnik3 branch October 3, 2014 23:16
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.

4 participants