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

use foundation base64 + async deprecations #103

Merged
merged 4 commits into from Mar 21, 2018

Conversation

2 participants
@tanner0101
Copy link
Member

tanner0101 commented Mar 21, 2018

  • use foundation for base64 coding
  • adds escape/unescape methods for base64 strings and data
  • adds deprecation warnings for some removed methods on Future

@tanner0101 tanner0101 added this to the 3.0.0-rc.2 milestone Mar 21, 2018

@tanner0101 tanner0101 self-assigned this Mar 21, 2018

@tanner0101 tanner0101 merged commit 152e9e3 into master Mar 21, 2018

1 of 2 checks passed

ci/circleci: linux-vapor Your tests failed on CircleCI
Details
ci/circleci: linux Your tests passed on CircleCI!
Details

@tanner0101 tanner0101 deleted the base64-foundation branch Mar 21, 2018

@MrMage

This comment has been minimized.

Copy link
Contributor

MrMage commented on Sources/Core/Data+Base64URL.swift in d0f2491 Mar 22, 2018

I think this adds four "====" if the input string's length is divisible by four. While it doesn't make much of a difference, I don't think we want that. Also, shouldn't things like this be covered with tests to spot such behavior? I had specifically provided tests in #102 that we could have re-used for this.

This comment has been minimized.

Copy link
Contributor

MrMage replied Mar 22, 2018

Never mind, I see this got fixed with 4f09ced. But my question about why this is not tested still stands.

This comment has been minimized.

Copy link
Contributor

MrMage replied Mar 22, 2018

Never mind again, I only now see that tests moved to https://github.com/vapor/core/blob/master/Tests/CoreTests/CoreTests.swift. Sorry!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment