Skip to content

Conversation

@cstjean
Copy link
Contributor

@cstjean cstjean commented Dec 7, 2016

I tried to match the tone from Base, but please let me know if anything sounds off.

@cstjean cstjean changed the title Add warning on unordered to ordered conversion Add warning on unordered to ordered conversion. Fixes #216 Dec 7, 2016
@codecov-io
Copy link

codecov-io commented Dec 7, 2016

Current coverage is 95.85% (diff: 71.42%)

Merging #237 into master will decrease coverage by 0.07%

@@             master       #237   diff @@
==========================================
  Files            30         30          
  Lines          2213       2220     +7   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
+ Hits           2123       2128     +5   
- Misses           90         92     +2   
  Partials          0          0          

Powered by Codecov. Last update 6846f65...eae3098

@kmsquire
Copy link
Member

@cstjean, sorry for the delay on this, but would you be able to add some tests?

@kmsquire
Copy link
Member

(Also, there is a minor conflict.)

@cstjean
Copy link
Contributor Author

cstjean commented Jan 25, 2017

Can I test the warnings? Or do you mean to make sure that is_ordered(Dict)==true?

@kmsquire
Copy link
Member

Just testing is_ordered (for all of the dictionaries for which it is defined) would be fine. Base runs julia with --depwarn=error, which could be caught, but I don't think packages are tested that way by default.

@cstjean cstjean force-pushed the pull-request/483460f4 branch from 483460f to bb1533b Compare January 26, 2017 15:38
@cstjean cstjean force-pushed the pull-request/483460f4 branch from bb1533b to eae3098 Compare January 26, 2017 15:44
@cstjean
Copy link
Contributor Author

cstjean commented Jan 26, 2017

Tested and rebased.

@kmsquire
Copy link
Member

Thanks!

@kmsquire kmsquire merged commit 52b74de into JuliaCollections:master Jan 26, 2017
@kmsquire
Copy link
Member

For testing the deprecation see JuliaLang/julia#20348.

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