Skip to content
This repository has been archived by the owner on Mar 23, 2021. It is now read-only.

Fix conversion of String->Int dictionary #53

Merged
merged 2 commits into from
Dec 5, 2016
Merged

Fix conversion of String->Int dictionary #53

merged 2 commits into from
Dec 5, 2016

Conversation

remko
Copy link
Contributor

@remko remko commented Sep 28, 2016

Failing test for #52

@czechboy0
Copy link
Collaborator

Can you please also add the test of the name to the manifest so that this test is ran on Linux as well? https://github.com/DanToml/Jay/blob/master/Tests/JayTests/FormattingTests.swift#L34

@remko
Copy link
Contributor Author

remko commented Sep 28, 2016

Oops, missed that. Fixed.

@remko
Copy link
Contributor Author

remko commented Sep 30, 2016

I did an explicit typecheck on Int instead of a cast. Let's see if the Linux builds still succeed.

Note that the inverse (typechecking for Bool) doesn't work; someInt is Bool returns true.

@remko
Copy link
Contributor Author

remko commented Sep 30, 2016

Note to self: run the full unit test suite before pushing :( Sorry for the noise, back to the drawing board.

@remko remko closed this Sep 30, 2016
@remko remko reopened this Sep 30, 2016
@remko remko changed the title Add failing test for String->Int dictionary Fix conversion of String->Int dictionary Sep 30, 2016
@remko
Copy link
Contributor Author

remko commented Sep 30, 2016

Not sure why the build is failing; the tests seem to pass.

@codecov-io
Copy link

codecov-io commented Oct 1, 2016

Current coverage is 88.32% (diff: 81.81%)

Merging #53 into master will decrease coverage by 0.80%

@@             master        #53   diff @@
==========================================
  Files            24         24          
  Lines          1914       1936    +22   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
+ Hits           1706       1710     +4   
- Misses          208        226    +18   
  Partials          0          0          

Powered by Codecov. Last update 828d8ac...267c7f0

@remko
Copy link
Contributor Author

remko commented Dec 2, 2016

Any updates on this? Is there anything I can do to get this merged?

@endocrimes endocrimes self-assigned this Dec 4, 2016
@endocrimes
Copy link
Owner

Looks like this will need a rebase then it's good to go, sorry for the delay!

@remko
Copy link
Contributor Author

remko commented Dec 4, 2016

@dantoml Thanks, done!

@endocrimes endocrimes merged commit 8331252 into endocrimes:master Dec 5, 2016
@vzsg vzsg mentioned this pull request Dec 6, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants