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

[AdSupportIOS] better module exports for AdSupportIOS #666

Closed
wants to merge 3 commits into from

Conversation

lvyile
Copy link
Contributor

@lvyile lvyile commented Apr 4, 2015

Before:

var AdSupportIOS = require('AdSupportIOS');

After:

var React = require('react-native');
var {
  //...
  AdSupportIOS,
  //...
} = React;

@lvyile
Copy link
Contributor Author

lvyile commented Apr 4, 2015

Why I test succeed on my device, but build failed on Travis CI ?

screen shot 2015-04-04 at 10 40 40 pm

@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 Apr 7, 2015
@ccheever
Copy link
Contributor

I think the code for this looks good but @lvyile you should probably squash your commits.

Someone from Facebook or more of a decision maker on the project probably needs to say whether

var AdSupportIOS = require('AdSupportIOS');

or

var React = require('react-native');
var {
  //...
  AdSupportIOS,
  //...
} = React;

is preferred. @brentvatne ?

@brentvatne
Copy link
Collaborator

I think the latter example is preferred in this case @ccheever.

One thing that we will want to do in addition to this is add some documentation in AdSupportIOS.js so that we can have it under the APIs section in the docs. If you're up for it @ccheever ;)

Agreed re:squashing, can you squash your commits into one @lvyile?

@brentvatne brentvatne changed the title better module exports for AdSupportIOS [AdSupportIOS] better module exports for AdSupportIOS Jun 1, 2015
@brentvatne brentvatne self-assigned this Jun 1, 2015
@nicklockwood
Copy link
Contributor

Seems like this now works as intended.

@facebook-github-bot
Copy link
Contributor

@lvyile updated the pull request.

This was referenced Feb 24, 2018
jfrolich pushed a commit to jfrolich/react-native that referenced this pull request Apr 22, 2020
* Bump bs-platform to 7.2.0

* Record as objects

* Update src/apis/NetInfo.md

Co-Authored-By: Max Thirouin <git@moox.io>

* Update src/apis/CameraRoll.re

Co-Authored-By: Max Thirouin <git@moox.io>

* Update documentation file VirtualizedSectionList

* Update documentation files

* Add missing bs.meth decorator

* Documentation fixes

* Documentation fixes

* Documentation updates

Co-authored-by: Max Thirouin <git@moox.io>
ayushjainrksh referenced this pull request in MLH-Fellowship/react-native Jul 2, 2020
mganandraj pushed a commit to mganandraj/react-native that referenced this pull request Jan 16, 2021
* change back to localhost

* boost path gradlew change

* whitespace
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.

6 participants