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.

Sign up for GitHub

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

Jump to bottom

[TIMOB-9111] iOS: Remove Base64Transcoder dependency, fix Base64-parity #9288

Merged
merged 8 commits into from Nov 15, 2017
Merged

[TIMOB-9111] iOS: Remove Base64Transcoder dependency, fix Base64-parity #9288

merged 8 commits into from Nov 15, 2017

Conversation

hansemannn
Copy link
Collaborator

@hansemannn hansemannn added this to the 7.0.0 milestone Aug 8, 2017
@sgtcoolguy
Copy link
Contributor

Looks like this now fails the base64#decode(Ti.Blob) unit test on iOS.

@hansemannn
Copy link
Collaborator Author

@sgtcoolguy Unit-test failure fixed.

@build build added the ios label Nov 2, 2017
@build
Copy link
Contributor

build commented Nov 2, 2017 •
edited

Fails
🚫

🔬 There are library changes, but no changes to the unit tests. That's OK as long as you're refactoring existing code, but will require an admin to merge this PR. Please see README.md#unit-tests for docs on unit testing.
Messages
📖

👍 Hey!, You deleted more code than you added. That's awesome!

Generated by 🚫 dangerJS

@hansemannn
Copy link
Collaborator Author

@vijaysingh-axway Save to review. @sgtcoolguy I didn't add new unit-tests, because the old ones should persist as they are.

@build build added the ios label Nov 13, 2017
vijaysingh-axway
vijaysingh-axway approved these changes Nov 14, 2017
View reviewed changes
Copy link
Contributor

@vijaysingh-axway vijaysingh-axway left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cr and FT passed.

@mukherjee2 mukherjee2 self-requested a review November 14, 2017 18:04
@build build added the ios label Nov 15, 2017
mukherjee2
mukherjee2 approved these changes Nov 15, 2017
View reviewed changes
Copy link
Contributor

@mukherjee2 mukherjee2 left a comment •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Node Version: 8.9.1
NPM Version: 5.5.1
Mac OS: 10.13
Appc CLI: 7.0.0-master.13
Appc CLI NPM: 4.2.11-2
Titanium SDK version: 7.0.0
Appcelerator Studio vers 4.10.0
Xcode 9.1/iOS 11.1 iPhone 7 Plus
Passed FR. I first reproduced with an affected version. I then tried it with the fix version and did not see "NEWLINE" in the alert, as expected.

@mukherjee2
Copy link
Contributor

@hansemannn can you look into the unit test failures?

@build build added the ios label Nov 15, 2017
@sgtcoolguy sgtcoolguy merged commit 831da45 into tidev:master Nov 15, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mukherjee2 mukherjee2 mukherjee2 approved these changes

@vijaysingh-axway vijaysingh-axway vijaysingh-axway approved these changes

Assignees
No one assigned
Labels
BREAKING CHANGES ⚠️ improvement ios parity 👯‍♀️
Projects
None yet
Milestone
7.0.0
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@hansemannn @sgtcoolguy @build @mukherjee2 @vijaysingh-axway @jquick-axway