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

Introduce kotlin and reimplement RN-timers idling resource #1085

Merged
merged 4 commits into from
Jan 14, 2019

Conversation

d4vidi
Copy link
Collaborator

@d4vidi d4vidi commented Dec 11, 2018

Description:

  1. Introduce Kotlin into detox core (lib)
  2. Reimplement the ReactNativeTimersIdlingResource in Kotlin, TDD.

@d4vidi d4vidi requested a review from rotemmiz December 11, 2018 11:30
@d4vidi
Copy link
Collaborator Author

d4vidi commented Dec 11, 2018

NOTE still need to check the effect kotiln would have on a potential detox .aar.
NOTE 2 still need to verify versioning collision and the minimal kotlin version.
In any case, I think we should publish this under a new major (or at least a minor)

@d4vidi d4vidi force-pushed the kotlin-rntimer-idling-resource branch from 795cc63 to b7db06c Compare December 11, 2018 12:07
@d4vidi d4vidi closed this Dec 11, 2018
@d4vidi d4vidi reopened this Dec 11, 2018
@d4vidi d4vidi force-pushed the kotlin-rntimer-idling-resource branch from b7db06c to f020871 Compare December 12, 2018 07:41
@d4vidi
Copy link
Collaborator Author

d4vidi commented Dec 12, 2018

Follow-up on the notes:

Seems there are plenty of Kotlin based .aar/.jar's out there. I've taken a deep look into some and it seems that while the delivered bytecode expects the presence of Kotlin elements (e.g. functions return kotlin.String), kotlin core is never bundled in. Rather, the hosting app should introduce Kotlin stdlib in its deps and - as long as the version is new enough (tested 1.1.0 in our case), it should all play nice together eventually.

As for versioning -- I worked out detox to default to 1.3.0 but to also expect the version to be defined by the hosting app by aligning a global called detoxKotlinVersion to their own kotlin version. We'll add that to the installation instructions and everything should be ok.

@d4vidi
Copy link
Collaborator Author

d4vidi commented Dec 12, 2018

Next up, we have the CI saying: > Plugin with id 'kotlin-android' not found. @yershalom and I are trying to figure out why gradle doesn't download it automatically.

@d4vidi
Copy link
Collaborator Author

d4vidi commented Dec 12, 2018

@rotemmiz you oughta change the project's settings to deny merging as long as checks haven't succeeded + view approved...

@yershalom
Copy link
Contributor

@d4vidi d4vidi force-pushed the kotlin-rntimer-idling-resource branch from f020871 to 368ebff Compare December 12, 2018 09:07
@d4vidi
Copy link
Collaborator Author

d4vidi commented Dec 12, 2018

@yershalom holy c***, giving it a try...

@d4vidi d4vidi force-pushed the kotlin-rntimer-idling-resource branch 9 times, most recently from 1366d8f to d26cbc7 Compare December 16, 2018 08:07
@d4vidi d4vidi force-pushed the kotlin-rntimer-idling-resource branch from d26cbc7 to 6e79f94 Compare January 8, 2019 12:46
@rotemmiz
Copy link
Member

@d4vidi what's the verdict? Were you able to solve it?

@d4vidi
Copy link
Collaborator Author

d4vidi commented Jan 10, 2019

@rotemmiz ios builds fail in very weird ways. I'm trying to figure where this is coming from, though in essence this PR has nothing to do with iOS at all.

@d4vidi
Copy link
Collaborator Author

d4vidi commented Jan 10, 2019

@yershalom connectivity issues? -

10:20:52 lerna info Installing external dependencies
... ( >4 minutes)
10:25:01 lerna ERR! execute callback with error
10:25:01 lerna ERR! Error: Command failed: npm install

'only other thing I can think of is changing the ver to 10.0.0, but that shouldn't matter, should it?

@d4vidi
Copy link
Collaborator Author

d4vidi commented Jan 13, 2019

I know what this is about. I'm taking care of it.

@d4vidi d4vidi force-pushed the kotlin-rntimer-idling-resource branch from a1cc05e to 93d6e33 Compare January 13, 2019 09:01
@d4vidi d4vidi force-pushed the kotlin-rntimer-idling-resource branch from 93d6e33 to dc20527 Compare January 13, 2019 09:01
@d4vidi d4vidi merged commit c3ab455 into master Jan 14, 2019
@d4vidi
Copy link
Collaborator Author

d4vidi commented Jan 14, 2019

Fixes issue #1115

@d4vidi d4vidi deleted the kotlin-rntimer-idling-resource branch January 14, 2019 16:02
@lock lock bot locked as resolved and limited conversation to collaborators Jan 17, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants