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

Support Swift 5.1 toolchains #29

Merged
merged 1 commit into from
Jun 4, 2019
Merged

Support Swift 5.1 toolchains #29

merged 1 commit into from
Jun 4, 2019

Conversation

JensAyton
Copy link
Contributor

@JensAyton JensAyton commented Jun 4, 2019

In older toolchains, == must have a dummy return statement. In newer ones, it must not. (Having one generates an unreachable code warning, and we treat warnings as errors.)

I’d like to formally deprecate NoEffect in favour of Never. Unfortunately, there is no way to make an availability attribute conditional on the version of Swift used by the consumer of the library, so as long as we support Swift 4.x (where Never isn’t Hashable or Equatable) we can’t do this.

If such a deprecation isn’t controversial, I guess we can document it as soft-deprecated.

@codecov
Copy link

codecov bot commented Jun 4, 2019

Codecov Report

Merging #29 into master will decrease coverage by 0.67%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #29      +/-   ##
==========================================
- Coverage   96.96%   96.29%   -0.68%     
==========================================
  Files          36       36              
  Lines         858      864       +6     
==========================================
  Hits          832      832              
- Misses         26       32       +6
Impacted Files Coverage Δ
MobiusCore/Source/NoEffect.swift 0% <0%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a4bf64b...62dbb0a. Read the comment docs.

@dflems
Copy link
Collaborator

dflems commented Jun 4, 2019

Gonna merge this. The rule that coverage can never ever ever be decreased (even by 0.68%) is a bit egregious.

@dflems
Copy link
Collaborator

dflems commented Jun 4, 2019

cc @jeppes

@dflems dflems merged commit 5730cc2 into master Jun 4, 2019
@dflems dflems deleted the xcode-11 branch June 4, 2019 12:25
@JensAyton
Copy link
Contributor Author

More importantly, it didn’t add any actual code and the code that is there can never be executed, so it’s quite hard to cover.

@dflems
Copy link
Collaborator

dflems commented Jun 4, 2019

I think it's that NoEffect.swift had no coverage before but now it has more lines of code (it counts the line with the #if and #else).

So coverage did go down, just barely.

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.

2 participants