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

Add back ImageEditingExample example #19972

Closed
wants to merge 7 commits into from

Conversation

gengjiawen
Copy link
Contributor

@gengjiawen gengjiawen commented Jun 29, 2018

Motivation

add back missing file.

Test Plan

pass all current ci.

Related PRs

none

Release Notes

[GENERAL] [INTERNAL] [RNTester] - Add back ImageEditingExample example.

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Jun 29, 2018
@react-native-bot react-native-bot added ✅Test Plan Missing Changelog This PR appears to be missing a changelog, or they are incorrectly formatted. labels Jun 29, 2018
@elicwhite
Copy link
Member

Woah, interesting find.

It looks like this file got accidentally messed up in this commit: 4a80dce#diff-9bff2388f38e11550ff316a7e78092bd

Instead of deleting this file could you add back the original version that was deleted in that commit?

@gengjiawen
Copy link
Contributor Author

@TheSavior I add it back. I check the file git history and see it it empty in first place, not realizing that it was deleted accidentally :)

@gengjiawen gengjiawen changed the title remove empty api Add back ImageEditingExample example Jun 30, 2018
@elicwhite
Copy link
Member

Thanks! Can you confirm that it works in RNTester now?

Also, it would be great if you migrated this file away from var to let/const. We have done that for most everything else in the codebase already but this file got missed since it was accidentally deleted.

@gengjiawen
Copy link
Contributor Author

Currently I only have an windows computer, but RNTester doesn't work on windows see #19974.
As for the var issue, most of RNTester still has the issue and use commonjs, I have a plan to migrate them all. But that's another story.

@gengjiawen
Copy link
Contributor Author

gengjiawen commented Jun 30, 2018

I test it on a mac via teamviewer after I grant permission , looks like ui rendered correctly

@facebook-github-bot facebook-github-bot added the Import Started This pull request has been imported. This does not imply the PR has been approved. label Jul 1, 2018
Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

@TheSavior is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@react-native-bot
Copy link
Collaborator

This pull request was closed by @gengjiawen in 6dcadca.

Once this commit is added to a release, you will see the corresponding version tag below the description at 6dcadca. If the commit has a single master tag, it is not yet part of a release.

@facebook facebook locked as resolved and limited conversation to collaborators Jul 2, 2018
@react-native-bot react-native-bot added the Merged This PR has been merged. label Jul 2, 2018
rozele pushed a commit to microsoft/react-native-windows that referenced this pull request Aug 17, 2018
Summary:
add back missing file.
pass all current ci.
none
 [GENERAL] [INTERNAL] [RNTester] - Add back ImageEditingExample example.
Closes facebook/react-native#19972

Differential Revision: D8711888

Pulled By: TheSavior

fbshipit-source-id: 14070bfbf747f7f59c4039981a9bc83c1c10862b
@hramos hramos removed Import Started This pull request has been imported. This does not imply the PR has been approved. labels Feb 6, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Merged This PR has been merged. Missing Changelog This PR appears to be missing a changelog, or they are incorrectly formatted.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants