Added base template for config ui.

Will build features from here.

BUG=730832
Review-Url: https://codereview.chromium.org/2923973003
Committed: https://github.com/luci/luci-py/commit/f52c4527495dc0c4fbb42def00117214784afc44

[cwpayton, 2017-06-07 18:17:49]

cwpayton@google.com changed reviewers:
+ hinoka@google.com, sergeyberezin@google.com

[cwpayton, 2017-06-07 18:17:51]

Template for luci config UI so that Ayana can also pull this code and we can
begin working together.

[cwpayton, 2017-06-07 18:18:21]

The CQ bit was checked by cwpayton@google.com to run a CQ dry run

[commit-bot: I haz the power, 2017-06-07 18:19:08]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-06-07 18:19:10]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-06-07 18:19:12]

Dry run: No L-G-T-M from a valid reviewer yet.
CQ run can only be started once the patch has received an L-G-T-M from a full
committer.
Even if an L-G-T-M may have been provided, it was from a non-committer,_not_ a
full super star committer.
Committers are members of the group "project-infra-committers".
Note that this has nothing to do with OWNERS files.

[Ryan Tseng, 2017-06-07 18:40:25]

+sean to make sure what I'm saying is sane.

Instead of checking in all of the dependencies, try to match what the swarming
ui does

1. Create a Makefile, there's an entry called "default" that runs "bower
install".
2. Add the .gitignore file and .bowerrc file, similar to
/appengine/swarming/ui/.bowerrc so that it ignores where the bower_components
are checked out
3. Just check in bower.json, and the vulcanized files if any.

[seanmccullough1, 2017-06-07 18:59:13]

On 2017/06/07 18:40:25, Ryan Tseng wrote:
> +sean to make sure what I'm saying is sane.
> 
> Instead of checking in all of the dependencies, try to match what the swarming
> ui does
> 
> 1. Create a Makefile, there's an entry called "default" that runs "bower
> install".
> 2. Add the .gitignore file and .bowerrc file, similar to
> /appengine/swarming/ui/.bowerrc so that it ignores where the bower_components
> are checked out
> 3. Just check in bower.json, and the vulcanized files if any.

Agree with all the above except for checking in the vulcanized files. Do not add
vulcanized files to the repo; they are deterministically generated from your
source element files (which *should* be in the repo) and should also be a make
target.

Leave bower_components dir out of the repo. Those dependencies are determined
entirely by the bower.json file (which *should* be in the repo) and can also be
regenerated deterministically.

(also, pls specify a bug in the CL description :)

[Sergey Berezin (google), 2017-06-07 19:14:55]

This looks good, modulo some comments.

CL description: I'd reformat for shorter title + longer description below:

  Added base template for config_service.

  Will build features from here.

If we don't yet have a tracking bug for the entire effort, let's create one and
link it to the CL.

Overall, I'm OK with vendoring (==copying) third-party dependencies for the time
being, but longer term we need to look into packaging them in CIPD. Let's also
file a bug for that.

https://codereview.chromium.org/2923973003/diff/1/appengine/config_service/ui...
File appengine/config_service/ui/index.html (right):

https://codereview.chromium.org/2923973003/diff/1/appengine/config_service/ui...
appengine/config_service/ui/index.html:7: <title>Luci Config UI</title>
"LUCI" is just one instance of the app; let's call it more generically
"Configuration Service". No need for "UI".

https://codereview.chromium.org/2923973003/diff/1/appengine/config_service/ui...
appengine/config_service/ui/index.html:8: <meta name="description" content="This
is a UI for the luci config service">
Same thing here: remove "luci" from the string.

https://codereview.chromium.org/2923973003/diff/1/appengine/config_service/ui...
File appengine/config_service/ui/manifest.json (right):

https://codereview.chromium.org/2923973003/diff/1/appengine/config_service/ui...
appengine/config_service/ui/manifest.json:2: "name": "Luci Config UI",
s/Luci//

https://codereview.chromium.org/2923973003/diff/1/appengine/config_service/ui...
File appengine/config_service/ui/src/luci-config-ui/luci-config-ui.html (right):

https://codereview.chromium.org/2923973003/diff/1/appengine/config_service/ui...
appengine/config_service/ui/src/luci-config-ui/luci-config-ui.html:3:
<dom-module id="luci-config-ui">
Same thing, let's rename the file and id to just "config-ui"

https://codereview.chromium.org/2923973003/diff/1/appengine/config_service/ui...
File appengine/config_service/ui/test/luci-config-ui/luci-config-ui_test.html
(right):

https://codereview.chromium.org/2923973003/diff/1/appengine/config_service/ui...
appengine/config_service/ui/test/luci-config-ui/luci-config-ui_test.html:45:
assert.equal(elementHeader.innerHTML, 'Hello new-prop1!');
Does this pass? I don't see how the header in the element would change with
prop1.

[Sergey Berezin (google), 2017-06-07 19:19:13]

On 2017/06/07 at 18:59:13, seanmccullough wrote:
> Leave bower_components dir out of the repo. Those dependencies are determined
entirely by the bower.json file (which *should* be in the repo) and can also be
regenerated deterministically.

Technically, they are not entirely deterministic, since some versions are
specified as "xxx or above". Do you have an opinion on packaging them in CIPD
instead?

[cwpayton, 2017-06-07 22:01:15]

PTAL

[Ryan Tseng, 2017-06-07 22:25:03]

lgtm + add bug 730832 to the description.

https://codereview.chromium.org/2923973003/diff/10001/appengine/config_servic...
File appengine/config_service/ui/test/config-ui/config-ui_test.html (right):

https://codereview.chromium.org/2923973003/diff/10001/appengine/config_servic...
appengine/config_service/ui/test/config-ui/config-ui_test.html:24: // TODO:
write tests for the config ui.
TODO(username,username):

[Sergey Berezin, 2017-06-07 22:27:01]

sergeyberezin@chromium.org changed reviewers:
+ sergeyberezin@chromium.org

[Sergey Berezin, 2017-06-07 22:27:02]

Much cleaner! One more comment, and please reformat the CL description per my
previous comment, and reference the bug that hinoka@ filed.

https://codereview.chromium.org/2923973003/diff/10001/appengine/config_servic...
File appengine/config_service/ui/Makefile (right):

https://codereview.chromium.org/2923973003/diff/10001/appengine/config_servic...
appengine/config_service/ui/Makefile:1: default:
In most files that allow that, we add a license header. Grab one from any Python
file and add here (update the year as necessary).

[cwpayton, 2017-06-07 22:58:15]

Description was changed from

==========
Added base template for luci-config ui. Will build features from here.

BUG=
==========

to

==========
Added base template for luci-config ui. Will build features from here.

BUG=730832
==========

[cwpayton, 2017-06-07 22:59:12]

Description was changed from

==========
Added base template for luci-config ui. Will build features from here.

BUG=730832
==========

to

==========
Will build features from here.

BUG=730832
==========

[cwpayton, 2017-06-07 23:07:30]

PTAL

[Ryan Tseng, 2017-06-07 23:10:20]

The CQ bit was checked by hinoka@google.com to run a CQ dry run

[commit-bot: I haz the power, 2017-06-07 23:10:25]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-06-07 23:12:35]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-06-07 23:12:36]

Dry run: This issue passed the CQ dry run.

[Sergey Berezin, 2017-06-08 00:05:59]

Great, LGTM + a few last comments (missing license headers in a few more files).
Thanks!

https://codereview.chromium.org/2923973003/diff/50001/appengine/config_servic...
File appengine/config_service/ui/index.html (right):

https://codereview.chromium.org/2923973003/diff/50001/appengine/config_servic...
appengine/config_service/ui/index.html:1: <!doctype html>
Please add a license header

https://codereview.chromium.org/2923973003/diff/50001/appengine/config_servic...
File appengine/config_service/ui/src/config-ui/config-ui.html (right):

https://codereview.chromium.org/2923973003/diff/50001/appengine/config_servic...
appengine/config_service/ui/src/config-ui/config-ui.html:1: <link rel="import"
href="../../bower_components/polymer/polymer.html">
Please add a license header here too. See how it's added in swarming/ui.

https://codereview.chromium.org/2923973003/diff/50001/appengine/config_servic...
File appengine/config_service/ui/test/config-ui/config-ui_test.html (right):

https://codereview.chromium.org/2923973003/diff/50001/appengine/config_servic...
appengine/config_service/ui/test/config-ui/config-ui_test.html:1: <!doctype
html>
Please add a license header.

[cwpayton, 2017-06-08 00:14:18]

The CQ bit was checked by cwpayton@google.com

[cwpayton, 2017-06-08 00:14:18]

The patchset sent to the CQ was uploaded after l-g-t-m from hinoka@google.com,
sergeyberezin@chromium.org
Link to the patchset: https://codereview.chromium.org/2923973003/#ps70001
(title: "Added license headers to necessary files.")

[commit-bot: I haz the power, 2017-06-08 00:14:29]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-06-08 00:16:03]

CQ is committing da patch.

Bot data: {"patchset_id": 70001, "attempt_start_ts": 1496880858480350,
"parent_rev": "a757a64e7cbadd0ff54eb1e2dcc22324ddb83849", "commit_rev":
"f52c4527495dc0c4fbb42def00117214784afc44"}

[commit-bot: I haz the power, 2017-06-08 00:16:06]

Description was changed from

==========
Will build features from here.

BUG=730832
==========

to

==========
Will build features from here.

BUG=730832

Review-Url: https://codereview.chromium.org/2923973003
Committed:
https://github.com/luci/luci-py/commit/f52c4527495dc0c4fbb42def00117214784afc44
==========

[commit-bot: I haz the power, 2017-06-08 00:16:08]

Committed patchset #5 (id:70001) as
https://github.com/luci/luci-py/commit/f52c4527495dc0c4fbb42def00117214784afc44

[turakulov, 2017-06-08 01:21:59]

turakulov@gmail.com changed reviewers:
+ turakulov@gmail.com

[turakulov, 2017-06-08 01:22:00]

lgtm

https://codereview.chromium.org/2923973003/diff/70001/appengine/config_servic...
File appengine/config_service/ui/README.md (right):

https://codereview.chromium.org/2923973003/diff/70001/appengine/config_servic...
appengine/config_service/ui/README.md:7: First, make sure you have the [Polymer
CLI](https://www.npmjs.com/package/polymer-cli) installed. Then run `polymer
serve` to serve your application locally.
Markdown files should not have lines longer than 80 chars
https://github.com/google/styleguide/blob/gh-pages/docguide/style.md#characte...

https://codereview.chromium.org/2923973003/diff/70001/appengine/config_servic...
File appengine/config_service/ui/src/config-ui/config-ui.html (right):

https://codereview.chromium.org/2923973003/diff/70001/appengine/config_servic...
appengine/config_service/ui/src/config-ui/config-ui.html:17: }
Why this style is defined in this file as opposed to index.html?
The html and body tags are defined there, not here

https://codereview.chromium.org/2923973003/diff/70001/appengine/config_servic...
appengine/config_service/ui/src/config-ui/config-ui.html:48: <image
src="../../images/google.png"/>
Is this a Google logo? I am not sure branding team would approve this. Better to
use Chromium logo

[nodir, 2017-06-08 14:49:01]

A commit subject "Will build features from here" in the global repo log
(https://github.com/luci/luci-py/commits/master) leaves a reader with questions
"what features?", "in which service?". Please try not to assume a reader has the
context.

Also please use "config_service: " prefix in CL descriptions to match existing
pattern
(https://github.com/luci/luci-py/commits/master/appengine/config_service)

Please use imperative style in commit messages, e.g. "config_service: add UI
boilerplate". "Fix foo", "Implement bar".

[nodir, 2017-06-13 01:08:59]

On 2017/06/08 01:22:00, turakulov wrote:
> lgtm
> 
>
https://codereview.chromium.org/2923973003/diff/70001/appengine/config_servic...
> File appengine/config_service/ui/README.md (right):
> 
>
https://codereview.chromium.org/2923973003/diff/70001/appengine/config_servic...
> appengine/config_service/ui/README.md:7: First, make sure you have the
[Polymer
> CLI](https://www.npmjs.com/package/polymer-cli) installed. Then run `polymer
> serve` to serve your application locally.
> Markdown files should not have lines longer than 80 chars
>
https://github.com/google/styleguide/blob/gh-pages/docguide/style.md#characte...
> 
>
https://codereview.chromium.org/2923973003/diff/70001/appengine/config_servic...
> File appengine/config_service/ui/src/config-ui/config-ui.html (right):
> 
>
https://codereview.chromium.org/2923973003/diff/70001/appengine/config_servic...
> appengine/config_service/ui/src/config-ui/config-ui.html:17: }
> Why this style is defined in this file as opposed to index.html?
> The html and body tags are defined there, not here
> 
>
https://codereview.chromium.org/2923973003/diff/70001/appengine/config_servic...
> appengine/config_service/ui/src/config-ui/config-ui.html:48: <image
> src="../../images/google.png"/>
> Is this a Google logo? I am not sure branding team would approve this. Better
to
> use Chromium logo

ping on these comments

[nodir, 2017-06-13 01:10:04]

On 2017/06/13 01:08:59, nodir wrote:
> On 2017/06/08 01:22:00, turakulov wrote:
> > lgtm
> > 
> >
>
https://codereview.chromium.org/2923973003/diff/70001/appengine/config_servic...
> > File appengine/config_service/ui/README.md (right):
> > 
> >
>
https://codereview.chromium.org/2923973003/diff/70001/appengine/config_servic...
> > appengine/config_service/ui/README.md:7: First, make sure you have the
> [Polymer
> > CLI](https://www.npmjs.com/package/polymer-cli) installed. Then run `polymer
> > serve` to serve your application locally.
> > Markdown files should not have lines longer than 80 chars
> >
>
https://github.com/google/styleguide/blob/gh-pages/docguide/style.md#characte...
> > 
> >
>
https://codereview.chromium.org/2923973003/diff/70001/appengine/config_servic...
> > File appengine/config_service/ui/src/config-ui/config-ui.html (right):
> > 
> >
>
https://codereview.chromium.org/2923973003/diff/70001/appengine/config_servic...
> > appengine/config_service/ui/src/config-ui/config-ui.html:17: }
> > Why this style is defined in this file as opposed to index.html?
> > The html and body tags are defined there, not here
> > 
> >
>
https://codereview.chromium.org/2923973003/diff/70001/appengine/config_servic...
> > appengine/config_service/ui/src/config-ui/config-ui.html:48: <image
> > src="../../images/google.png"/>
> > Is this a Google logo? I am not sure branding team would approve this.
Better
> to
> > use Chromium logo
> 
> ping on these comments

The comments were addressed in https://codereview.chromium.org/2935763003/