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

Jump to bottom

[expo-gl][web] Allow createElement & unstable_createElement … #7995

Merged
merged 1 commit into from May 6, 2020
Merged

[expo-gl][web] Allow createElement & unstable_createElement … #7995

merged 1 commit into from May 6, 2020

Conversation

wood1986
Copy link
Contributor

fix #6384

EvanBacon
EvanBacon requested changes Apr 24, 2020
View reviewed changes
Copy link
Contributor

@EvanBacon EvanBacon left a comment

Choose a reason for hiding this comment

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

Perhaps you could extract the required functionality from those methods

@EvanBacon EvanBacon self-assigned this Apr 24, 2020
@EvanBacon EvanBacon added the Platform: web Using Expo in the browser label Apr 24, 2020
@EvanBacon EvanBacon removed their assignment Apr 24, 2020
@wood1986
Copy link
Contributor Author

How about this?

@wood1986 wood1986 force-pushed the wood1986-patch-1 branch 3 times, most recently from 17d7131 to 64fc70a Compare April 27, 2020 01:25
@wood1986 wood1986 requested a review from EvanBacon April 27, 2020 01:30
@wood1986
Copy link
Contributor Author

Any updates?

@wood1986
Copy link
Contributor Author

Hey guys, would you mind get this review? As for the errors, I am not sure if it is come from me.

bbarthec
bbarthec suggested changes May 4, 2020
View reviewed changes
Copy link
Contributor

@bbarthec bbarthec left a comment

Choose a reason for hiding this comment

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

LGTM 💪👌👏
Regarding CI errors, please do the followings:

  • rebase on the current master
  • rebuild the package affected by your changes (packages/expo-gl) by invoking following command in this package directory:
    • yarn build - rebuild .d.ts files and .js files
  • ensure correct format of your changes, by running yarn lint

@wood1986 wood1986 force-pushed the wood1986-patch-1 branch 4 times, most recently from 13e3795 to 08cf63a Compare May 4, 2020 23:26
@wood1986
Copy link
Contributor Author

wood1986 commented May 4, 2020 •
edited

It is fixed. As for the error in Public Expo Web, it requires some credential

@wood1986 wood1986 requested a review from bbarthec May 4, 2020 23:51
@bbarthec
Copy link
Contributor

bbarthec commented May 5, 2020

One missing thing in your PR is CHANGELOG entry 😉 Please add CHENGELOG entry in correct package according to the style from the main CHANGELOG file and we'll be ready to merge it 😊

@wood1986
Copy link
Contributor Author

wood1986 commented May 5, 2020

One missing thing in your PR is CHANGELOG entry 😉 Please add CHENGELOG entry in correct package according to the style from the main CHANGELOG file and we'll be ready to merge it 😊

Maybe you guys should use conventional-changelog spec and conventional-changelog

@wood1986
Copy link
Contributor Author

wood1986 commented May 5, 2020

Not sure if it is correct

bbarthec
bbarthec suggested changes May 5, 2020
View reviewed changes
packages/expo-gl/CHANGELOG.md Outdated Show resolved Hide resolved
@wood1986 wood1986 requested a review from bbarthec May 5, 2020 16:57
@bbarthec bbarthec changed the title fix(web): Allow createElement & unstable_createElement usage [expo-gl][web] Allow createElement & unstable_createElement … May 6, 2020
@bbarthec bbarthec merged commit 49f1b9e into expo:master May 6, 2020
@wood1986
Copy link
Contributor Author

wood1986 commented May 6, 2020

Thanks, when will you publish a new package?

@jarvisluong jarvisluong mentioned this pull request Jun 2, 2020
Closed
@EvanBacon EvanBacon mentioned this pull request Jun 5, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tsapeta tsapeta Awaiting requested review from tsapeta tsapeta is a code owner

@EvanBacon EvanBacon Awaiting requested review from EvanBacon

@bbarthec bbarthec Awaiting requested review from bbarthec

Assignees
No one assigned
Labels
Platform: web Using Expo in the browser
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Can't explicitly import react-native-web in a Snack
3 participants
@wood1986 @bbarthec @EvanBacon