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

RFC: syntax update #29

Merged
merged 12 commits into from
Jan 26, 2017
Merged

RFC: syntax update #29

merged 12 commits into from
Jan 26, 2017

Conversation

felixrehren
Copy link
Contributor

  • Syntax updating for compatibility with 0.6.0-dev
    • replaced KVPair{K,V} type with standard Pair{K,V}
  • changed factcheck to @test usage

Almost compatible with 0.5.0, except some problems marked in the test suite. Not sure what to do here:

julia> PersistentSet(Integer[1,2,3])
ERROR: MethodError: Cannot `convert` an object of type Pair{Int64,Void} to an object of type Pair{Integer,Void}

or here:

julia> append(vec(1:31), 32) == vec(1:32)
true
julia> @test append(vec(1:31), 32) == vec(1:32)
[...] ERROR: MethodError: no method matching unsafe_length(::Int64)

Syntax updating for compatibility with 0.6.0-dev

Almost compatible with 0.5.0, except some problems marked in the test suite. Not sure what to do here:

    julia> PersistentSet(Integer[1,2,3])
    ERROR: MethodError: Cannot `convert` an object of type Pair{Int64,Void} to an object of type Pair{Integer,Void}

or here:

    julia> append(vec(1:31), 32) == vec(1:32)
    true
    julia> @test append(vec(1:31), 32) == vec(1:32)
    [...] ERROR: MethodError: no method matching unsafe_length(::Int64)
@felixrehren
Copy link
Contributor Author

Travis passes for nightly, runs into the above errors for 0.5.0, and fails on 0.4.0 because @testset is not defined there

@felixrehren felixrehren changed the title RFC: syntax update RFH: syntax update Jan 16, 2017
@felixrehren
Copy link
Contributor Author

Would love help making this functional. Ping @shashi as the last person to commit here

I can unwind parts of this PR if too aggressive?

@tkelman
Copy link
Contributor

tkelman commented Jan 25, 2017

You could either depend on BaseTestNext on 0.4, or bump the minimum Julia version to 0.5.

Change `Integer` type to `Int64` and test covariance of dicts only for `VERSION > v"0.6"`
@felixrehren
Copy link
Contributor Author

felixrehren commented Jan 25, 2017

Thanks @tkelman. I took the easy way out, requiring 0.5 and avoiding the tests mentioned above that were breaking < v"0.6". Hope build is successful now ...

@felixrehren felixrehren changed the title RFH: syntax update RFC: syntax update Jan 25, 2017
@shashi shashi merged commit 6a6b6cd into JuliaCollections:master Jan 26, 2017
@shashi
Copy link
Contributor

shashi commented Jan 26, 2017

Thanks @felixrehren! This is a great improvement.

@tkelman
Copy link
Contributor

tkelman commented Jan 26, 2017

Since this is almost a complete rewrite, you should probably double-check that it won't break Arbiter or Patchwork, its reverse-dependencies.

@shashi
Copy link
Contributor

shashi commented Jan 27, 2017

Not much has been rewritten, but code has been moved around. Just ran tests for Patchwork and Arbiter on 0.5 - all green.

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.

3 participants