JuliaIO / LibExpat.jl

Julia interface to the Expat XML parser library
Other
9 stars 32 forks source link

Fix deprecation of int(x) in Julia 0.4 #25

Closed axsk closed 9 years ago

axsk commented 9 years ago

Doing a Pkg.update() on the newest Julia nightly I receive a LOT of

WARNING: int(x) is deprecated, use Int(x) instead.
 in cdata at C:\cygwin64\home\Alex\.julia\v0.4\LibExpat\src\LibExpat.jl:166

slowing down the update process so much, that I actually canceled it after 10 minutes.

The provided ad-hoc patch solved this issue. I did not check for other deprecated code, as I am not informed on these changes.

WARNING: I do not know if this works with Julia 0.3.

garborg commented 9 years ago

Thanks, @axsk -- if you make the changes I marked inline, we can get your PR merged.

garborg commented 9 years ago

If you haven't seen it yet, running Pkg.test("LibExpat") after making your initial changes would catch the third change, and running it again on 0.3 would confirm whether or not adjustments were needed for 0.3 compatibility.

garborg commented 9 years ago

When you make the edits, please squash them to one atomic commit (so there's not an intermediate commit that doesn't work on 0.3). Thanks!

tkelman commented 9 years ago

Shouldn't Travis be running here?

garborg commented 9 years ago

@tkelman It should be (as of #23). I just flipped off Travis's 'Build only if .travis.yml is present' switch, though that shouldn't make a difference. The other switches are on.

@amitmurthy Could you check that this package (GitHub) Settings -> Webhooks & Services has Travis CI present with a green checkmark in the Services section?

That's supposed to be taken care of when you turned on Travis -- not sure how easiest to fix it if not, but @tkelman may know.

amitmurthy commented 9 years ago

This is what it looks like now. I don't see a green checkmark.

image

tkelman commented 9 years ago

That looks like it's disabled for some reason, does it let you turn it back on if you click edit?

garborg commented 9 years ago

(Maybe it's just a little unchecked checkbox at the bottom titled 'Active'.)

amitmurthy commented 9 years ago

The green checkmark is there now. Maybe the Travis/Github setup process was taking time.

garborg commented 9 years ago

And #26 (a fresh, just-to-test-travis PR) triggered a build. I'm wondering if there was a problem with the "only when .travis.yml is present" option.

amitmurthy commented 9 years ago

No, sorry I wasn't clear. Travis CI was not present as an added Service. I added it and expected to see a checkmark, which I didn't find. I guess #26 triggered a build and so we are all good now.

garborg commented 9 years ago

Ah, gotcha -- thanks!

On Wed, Mar 25, 2015 at 10:05 AM, Amit Murthy notifications@github.com wrote:

No, sorry I wasn't clear. Travis CI was not present as an added Service. I added it and expected to see a checkmark, which I didn't find. I guess #26 https://github.com/amitmurthy/LibExpat.jl/pull/26 triggered a build and so we are all good now.

— Reply to this email directly or view it on GitHub https://github.com/amitmurthy/LibExpat.jl/pull/25#issuecomment-86096484.

coveralls commented 9 years ago

Coverage Status

Changes Unknown when pulling 78d32e5513f5b4c5925db8f8e7ba15530babd0a1 on axsk:patch-1 into \ on amitmurthy:master**.

garborg commented 9 years ago

Hmmm... tested before committing and this passed on 0.3 and 0.4 locally.

I believe the the failure on 0.4 on Travis is the same failure that keeps OpenStreetMap.jl failing (0.4, on Travis, not locally) failing downstream: https://travis-ci.org/tedsteiner/OpenStreetMap.jl/jobs/49702311 https://travis-ci.org/tedsteiner/OpenStreetMap.jl/jobs/49701904 https://travis-ci.org/tedsteiner/OpenStreetMap.jl/jobs/47683838

Anyone seen similar elsewhere?

amitmurthy commented 9 years ago

The same issue on Travis has been present for HTTPClient.jl https://travis-ci.org/JuliaWeb/HTTPClient.jl/jobs/53431520 - I have been unable to figure it out. Happens on a ccall . One difference I noticed was that Travis uses AMD CPUs while local testing (which passed) were all Intel.

I think you can go ahead and merge if it passes locally. Current master is also failing on Travis I guess.