Skip to content

Maintenance: Unify examples#198

Merged
rotemmiz merged 6 commits into
wix-incubator:masterfrom
mkuczera:unify-examples
Apr 14, 2018
Merged

Maintenance: Unify examples#198
rotemmiz merged 6 commits into
wix-incubator:masterfrom
mkuczera:unify-examples

Conversation

@mkuczera

@mkuczera mkuczera commented Apr 3, 2018

Copy link
Copy Markdown
Contributor

As discuessed, unified real life examples and examples. Also prepare test setup

@rotemmiz rotemmiz changed the title Unify examples Maintenance: Unify examples Apr 3, 2018

@rotemmiz rotemmiz left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Great job! This looks super neat, thanks!
I had a few minor issues, nothing big.

Comment thread package.json Outdated
"react": ">= 15.4.1",
"react-native": ">= 0.40.0"
"react": "*",
"react-native": "*"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The project requires RN>40 IIRC

Comment thread playground/android/build.gradle Outdated
}
dependencies {
classpath 'com.android.tools.build:gradle:2.3.0'
classpath 'com.android.tools.build:gradle:2.2.3'

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

com.android.tools.build:gradle:2.2.3 is VERY old. Let's please move to 3.0.1 or at least stay at 2.3.0

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Detox will not work with that gradle plugin version, it requires 3.x.x

distributionBase=GRADLE_USER_HOME
distributionPath=wrapper/dists
zipStoreBase=GRADLE_USER_HOME
zipStorePath=wrapper/dists
distributionUrl=https\://services.gradle.org/distributions/gradle-3.3-all.zip

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Same here please, let's move forward, not backwards.

@rotemmiz rotemmiz mentioned this pull request Apr 3, 2018
@mkuczera

mkuczera commented Apr 4, 2018

Copy link
Copy Markdown
Contributor Author

@rotemmiz i updated the PR. Thanks for reviewing ;)

@rotemmiz rotemmiz left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

com.android.tools.build:gradle:3.0.1 requires gradle-4.1.
Were you able to run the Android playground app with the current configuration?

@mkuczera

mkuczera commented Apr 4, 2018

Copy link
Copy Markdown
Contributor Author

Strange. Ran it this morning and everything was fine. Deleted the app from the phone and ran it again and it was not able to build. I will take this as a task to to everytime before pushing.

  • Added google Repo to buildscripts
  • Added package.json run-android
  • Set gradle version to 4.1

@rotemmiz

rotemmiz commented Apr 4, 2018

Copy link
Copy Markdown
Contributor

One more thing that needs to be verified is what npm packs inside the package with the new structure.
I suggest you run npm pack and then tar -tf react-native-interactable-{version}.tgz, and adjust .npmignore accordingly.

@mkuczera

mkuczera commented Apr 5, 2018

Copy link
Copy Markdown
Contributor Author

Perfect. Thanks for the hint. Never had a look on this to be honest. Updated the npmignore accordingly to not include e2e and playground.

Just one question beside: What´s the purpose of the
package/.ghp/wix.json

Just saw it in every Wix project. Just internal declaration?

@rotemmiz rotemmiz merged commit a147754 into wix-incubator:master Apr 14, 2018
@rotemmiz

Copy link
Copy Markdown
Contributor

Thanks!

@IanCStewart IanCStewart mentioned this pull request Aug 7, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants