feat: ML Kit Quickstart Kotlin #639

thatfiredev · 2018-09-07T01:59:48Z

fixed #635

By the time I realized this wasn't one of the simplest quickstarts it was too late. It should probably be one of the last translations. My apologies

...gle/firebase/samples/apps/mlkit/kotlin/cloudtextrecognition/CloudTextRecognitionProcessor.kt

samtstern · 2018-09-07T16:13:54Z

@rosariopfernandes thanks for jumping on this! Since the build is red here I went and reviewed the auth PR by @the-dagger first.

It may be useful for you to participate in that review as well (as an observer and a reviewer), I am sure the decisions we make over there will impact the work you're doing.

mlkit/app/src/main/java/com/google/firebase/samples/apps/mlkit/kotlin/CameraSource.kt

mlkit/app/src/main/java/com/google/firebase/samples/apps/mlkit/kotlin/FrameMetadata.kt

samtstern · 2018-09-10T19:58:25Z

mlkit/app/src/main/java/com/google/firebase/samples/apps/mlkit/kotlin/LivePreviewActivity.kt

+        if (preview == null) {
+            Log.d(TAG, "Preview is null")
+        }
+        graphicOverlay = findViewById<View>(R.id.fireFaceOverlay) as GraphicOverlay


optional: should we use synthetic binding instead of findViewById?

Since the activity xml file uses the java.GraphicOverlay class, synthetic binding automatically casts it to the GraphicOverlay class from the java package.

An alternative to findViewById would be to create a second xml file for that activity and use the kotlin.GraphicOverlay class. But that would make this app different from the others.

Your thoughts?

Ohh that's an interesting edge case! I'd say in this case we can leave the dependency on java then, thanks for pointing that out.

@samtstern This actually leads to runtime errors, since java.GraphicOverlay can't be cast to kotlin.GraphicOverlay.
I suggest we keep these classes written in java, but move them to a common package at the root of the quickstart so that they can be used in both the java and kotlin packages. The only classes written in kotlin would be the ones that actually make use of the ML Kit SDK.
I believe @the-dagger 's lint check would allow this.

@samtstern I need your thoughts on my comment above.

@rosariopfernandes apologies for not responding, moving to a .common package sounds like the right idea

Thanks, will do so.

mlkit/app/src/main/java/com/google/firebase/samples/apps/mlkit/kotlin/StillImageActivity.kt

...rc/main/java/com/google/firebase/samples/apps/mlkit/kotlin/barcodescanning/BarcodeGraphic.kt

… into mlkit-kotlin

thatfiredev · 2018-09-10T23:01:16Z

@samtstern Thanks for your review here. Since there are a lot of files to be changed here (to remove all the !! and use lateinit on activities), I'd like to work on the other apps and let this be my last one (if you don't mind). Then I'll let you know when I update the PR with all the review feedback so that it can go through that last review before merge.
Is that ok?

samtstern · 2018-09-10T23:44:40Z

@rosariopfernandes ok sounds good to me, let's put this one on hold.

… into mlkit-kotlin

the-dagger

Looks good, minor styling comments.

mlkit/app/src/main/java/com/google/firebase/samples/apps/mlkit/kotlin/LivePreviewActivity.kt

mlkit/app/src/main/java/com/google/firebase/samples/apps/mlkit/kotlin/ChooserActivity.kt

mlkit/app/src/main/java/com/google/firebase/samples/apps/mlkit/kotlin/LivePreviewActivity.kt

the-dagger · 2018-09-13T23:02:57Z

...va/com/google/firebase/samples/apps/mlkit/kotlin/barcodescanning/BarcodeScanningProcessor.kt

+            barcodes: List<FirebaseVisionBarcode>,
+            frameMetadata: FrameMetadata,
+            graphicOverlay: GraphicOverlay) {
+        graphicOverlay.clear()


nit : indentation here

...va/com/google/firebase/samples/apps/mlkit/kotlin/barcodescanning/BarcodeScanningProcessor.kt

...n/java/com/google/firebase/samples/apps/mlkit/kotlin/cloudimagelabeling/CloudLabelGraphic.kt

...m/google/firebase/samples/apps/mlkit/kotlin/cloudlandmarkrecognition/CloudLandmarkGraphic.kt

...main/java/com/google/firebase/samples/apps/mlkit/kotlin/custommodel/CustomImageClassifier.kt

thatfiredev · 2018-09-14T00:23:50Z

@the-dagger Thanks a lot for reviewing this.

@samtstern I believe this one is ready to go.

feat: ML Kit Quickstart Kotlin

6049cdc

thatfiredev commented Sep 7, 2018

View reviewed changes

...gle/firebase/samples/apps/mlkit/kotlin/cloudtextrecognition/CloudTextRecognitionProcessor.kt Show resolved Hide resolved

fix: fix build

3280ffc

samtstern reviewed Sep 10, 2018

View reviewed changes

thatfiredev added 2 commits September 10, 2018 23:07

Merge branch 'master' of https://github.com/firebase/quickstart-android…

5295098

… into mlkit-kotlin

Merge branch 'master' of https://github.com/firebase/quickstart-android…

31c28bb

… into mlkit-kotlin

thatfiredev added 5 commits September 13, 2018 23:33

refactor: Create .common package

6c8f007

chore: minor import changes

8168373

Merge branch 'master' of https://github.com/firebase/quickstart-android…

0e83cf6

… into mlkit-kotlin

chore: Add Lint

2d774d1

style: final Kotlin improvements

81213d9

the-dagger reviewed Sep 13, 2018

View reviewed changes

style: changes after feedback from review

a1d96b6

the-dagger approved these changes Sep 14, 2018

View reviewed changes

samtstern approved these changes Sep 14, 2018

View reviewed changes

samtstern merged commit 3c2a168 into firebase:master Sep 14, 2018

thatfiredev deleted the mlkit-kotlin branch September 14, 2018 18:13

This was referenced May 29, 2020

[Snyk] Security upgrade firebase-functions from 0.9.0 to 3.6.2 WittmannJ/quickstart-android#5

Open

[Snyk] Security upgrade firebase-functions from 0.9.0 to 3.6.2 Jeremip11/quickstart-android#4

Open

lsdace30095 mentioned this pull request Dec 22, 2022

[Snyk] Security upgrade firebase-functions from 0.9.0 to 3.6.2 lsdace30095/quickstart-android#12

Open

WittmannJ mentioned this pull request Dec 22, 2022

[Snyk] Security upgrade firebase-functions from 0.9.0 to 3.6.2 WittmannJ/quickstart-android#12

Open

Jeremip11 mentioned this pull request Dec 23, 2022

[Snyk] Fix for 4 vulnerabilities Jeremip11/quickstart-android#11

Open

lsdace30095 mentioned this pull request Dec 23, 2022

[Snyk] Fix for 4 vulnerabilities lsdace30095/quickstart-android#13

Open

snyk-bot mentioned this pull request Dec 23, 2022

[Snyk] Fix for 4 vulnerabilities KatanMichael/quickstart-android#7

Open

feat: ML Kit Quickstart Kotlin #639

feat: ML Kit Quickstart Kotlin #639

Uh oh!

Conversation

thatfiredev commented Sep 7, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

samtstern commented Sep 7, 2018

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

samtstern Sep 10, 2018

Choose a reason for hiding this comment

Uh oh!

thatfiredev Sep 10, 2018

Choose a reason for hiding this comment

Uh oh!

samtstern Sep 10, 2018

Choose a reason for hiding this comment

Uh oh!

thatfiredev Sep 11, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

thatfiredev Sep 13, 2018

Choose a reason for hiding this comment

Uh oh!

samtstern Sep 13, 2018

Choose a reason for hiding this comment

Uh oh!

thatfiredev Sep 13, 2018

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

thatfiredev commented Sep 10, 2018

Uh oh!

samtstern commented Sep 10, 2018

Uh oh!

the-dagger left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

the-dagger Sep 13, 2018

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

thatfiredev commented Sep 14, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

thatfiredev commented Sep 7, 2018 •

edited

Loading

thatfiredev Sep 11, 2018 •

edited

Loading