Skip to content

Conversation

@samtstern
Copy link
Contributor

Fixes #628

This was a pretty big one!

@samtstern samtstern changed the title Ss database kotlin Convert database quickstart to Kotlin Sep 10, 2018
Copy link
Member

@thatfiredev thatfiredev left a comment

Choose a reason for hiding this comment

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

Nicely done. Just got a few comments

Copy link
Contributor

@the-dagger the-dagger left a comment

Choose a reason for hiding this comment

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

Added some comments, mostly nits

Copy link
Contributor

@the-dagger the-dagger left a comment

Choose a reason for hiding this comment

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

Nothing major, good to merge!

Change-Id: Ifcb9ce439a1c3e383e11f75b3c5e159af1e5c6b8
Change-Id: I0f9de2781911366c29045fa1a312e97e019d5632
Change-Id: Iec50975804fd214827c63f36a0e617bf0509f64d
Change-Id: I9ac66f5c5f1bfe085b76e570a5c26c0d5dc785b0
Change-Id: Ic4b4315a504784432ffbf244e3086fe3c7b92590
@samtstern
Copy link
Contributor Author

@the-dagger hey the lint check found some errors in this PR (after I rebased) so, success!

Change-Id: Ifcd9fda9e72eb9634b5c619f08b8fdf06274cb26
@the-dagger
Copy link
Contributor

Woot! 💯
I too noticed some super helpful errors while converting the app-indexing quickstarters.
I think that we should add more lint checks, also what do you think about adding Codacy to the project?
https://www.codacy.com/

I've used it earlier and it's super helpful for monitoring common mistakes in pull requests.

@samtstern
Copy link
Contributor Author

samtstern commented Sep 12, 2018 via email

@the-dagger
Copy link
Contributor

Ah I see.
Makes sense.

@samtstern
Copy link
Contributor Author

@the-dagger yeah they use the broad permissions:
image

I'm sure they never would, but granting read and write access means they could nuke all of our samples and snippets! So for now, no Codacy.

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.

3 participants