Skip to content

Omit all line terminators for ImageStore.getBase64ForTag#11155

Closed
leeight wants to merge 1 commit into
react:masterfrom
leeight:fix-11142
Closed

Omit all line terminators for ImageStore.getBase64ForTag#11155
leeight wants to merge 1 commit into
react:masterfrom
leeight:fix-11142

Conversation

@leeight

@leeight leeight commented Nov 27, 2016

Copy link
Copy Markdown
Contributor

ImageStore.getBase64ForTag result should omit the line terminators, and fix #11142

Test plan (required)

Manually.

@facebook-github-bot

Copy link
Copy Markdown
Contributor

By analyzing the blame information on this pull request, we identified @AaaChiuuu to be a potential reviewer.

@facebook-github-bot facebook-github-bot added GH Review: review-needed CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. labels Nov 27, 2016
@mkonicek

Copy link
Copy Markdown
Contributor

"Manually" is not a test plan. You'll need to explain how exactly you tested this - what commands you ran, what their output was.

@lacker

lacker commented Dec 15, 2016

Copy link
Copy Markdown
Contributor

I think better than a "test plan" would just be a unit test ensuring that this works correctly.

@hramos

hramos commented Jan 19, 2017

Copy link
Copy Markdown
Contributor

Ping @leeight, see lacker's feedback above.

@FnTm

FnTm commented May 8, 2017

Copy link
Copy Markdown
Contributor

I'll take a stab at coming up with some tests for this

@FnTm

FnTm commented May 9, 2017

Copy link
Copy Markdown
Contributor

I didn't know how to add/change commits for this PR, so I created a new one that fixes this problem #13856

@hramos hramos closed this May 15, 2017
facebook-github-bot pushed a commit that referenced this pull request Oct 10, 2017
Summary:
FIX #11142

This fixes #11142 and supersedes #11155 as I was unsure how to add/change commits in that PR.

Wrote up a bunch of unit tests for the ImageStore module. The added tests showed that there was indeed a problem with the flags used for the Base64OutputStream, and they also show that that has been fixed now.
Closes #13856

Differential Revision: D6017764

Pulled By: shergin

fbshipit-source-id: adf667dc722ddfe31449afd8cd20a0a192eacff6
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

base64 string got from ImageEditor.cropImage(uri) have linefeed after each 76 characters.

6 participants