Skip to content
This repository was archived by the owner on Sep 10, 2022. It is now read-only.

Comments

Add example android app able to make GetEntry requests#2

Closed
AMarcedone wants to merge 10 commits intogoogle:masterfrom
AMarcedone:bazelProjectSetup
Closed

Add example android app able to make GetEntry requests#2
AMarcedone wants to merge 10 commits intogoogle:masterfrom
AMarcedone:bazelProjectSetup

Conversation

@AMarcedone
Copy link
Contributor

Not ready for merge.
Responding to feedback from #1, this PR does not check in any IDE files and uses bazel instead of gradle to build.

Early feedback appreciated. Especially on project setup/directory structure etc.

To test, update the GOPATH and ANDROID_HOME in the .bazelrc file, then run bazel mobile-install exampleapp.
Depends on google/keytransparency#736 (which in turn depends on another trillian PR)

TODO: remove reference to my personal machine in the WORKSPACE file.

@AMarcedone AMarcedone added the WIP label Aug 15, 2017
exampleapp/BUILD Outdated
@@ -0,0 +1,13 @@
android_binary(
name = "exampleapp",
srcs = glob(["src/main/java/com/google/keytransparency/exampleapp/*.java"]),
Copy link
Contributor

Choose a reason for hiding this comment

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

We prefer to not use glob in BUILD files if we can.
Would you mind breaking these files out into a list?

@gdbelvin
Copy link
Contributor

I hope this is not pedantic, but it would great if you could commit the library component as it's own PR? This PR is great, but it contains both the app and the library.

@@ -0,0 +1,3 @@
-----BEGIN PUBLIC KEY-----
Copy link
Contributor

Choose a reason for hiding this comment

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

These can now be pulled directly from v1/domain/info for a simpler configuration

@@ -0,0 +1,28 @@
android_library(
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like this file could use a format pass with the buildifier formatter
go get github.com/bazelbuild/buildtools/buildifier

outs = ["keytransparency_gobind.aar"],
tags=["local"],
local = 1,
cmd = "gomobile bind -target android -o $@ -javapkg com.google.keytransparency.gobind github.com/google/keytransparency/core/client/gobindClient github.com/google/keytransparency/core/client/kt github.com/google/keytransparency/core/crypto/commitments",
Copy link
Contributor

Choose a reason for hiding this comment

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

should these be listed as the sources?

Copy link
Contributor Author

@AMarcedone AMarcedone Aug 15, 2017

Choose a reason for hiding this comment

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

I think so (so that the library is rebuilt when they change), but they live in the $GOPATH directory which is not part of the workspace.
Also, these packages in turn depend on other go packages (like trillian), so I would have to list the whole GOPATH as source?

Also, we need to decide what to do with vendoring....

Copy link
Contributor

@gdbelvin gdbelvin left a comment

Choose a reason for hiding this comment

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

This is such a great start.
Please pair this PR down to just be the java library.

@AMarcedone
Copy link
Contributor Author

I will move the android exampleapp part to a separate PR.

@AMarcedone AMarcedone changed the title Setup Bazel workspace and modules. Create Android app able to make GetEntry requests Add example android app able to make GetEntry requests Aug 17, 2017
@AMarcedone
Copy link
Contributor Author

The library is now in a separate PR #3. This is still a WIP, but it should build and run if anybody is curious to test.

ktandroidtest

@googlebot
Copy link

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for the commit author(s). If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and have the pull request author add another comment and the bot will run again.

@AMarcedone AMarcedone removed the WIP label Aug 24, 2017
@AMarcedone
Copy link
Contributor Author

Closed. The code was merged through a different PR #6

@AMarcedone AMarcedone closed this Aug 24, 2017
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