New issue
Advanced search Search tips

Issue 674660 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner: ----
Closed: Jan 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: ----



Sign in to add a comment

Split up som-app.html

Project Member Reported by zhangtiff@chromium.org, Dec 15 2016

Issue description

Not sure what the best way to do this is. I've tried a few times to get the alerts-list separated out into a separate view, but there's more cross-dependencies than I expected and it gets really hairy really quickly. 

But som-app.html is rather unwieldy right now. Perhaps a simple change if we wanted could be to just split up the HTML, CSS, and JS for just this file, but it would be a bit odd to do this for only one element (the other elements are generally too small for this to make sense). 

Might want to look into how other application structure their Polymer. Passing data and events between elements can seem to get really messy really quickly. 
 
I talked to someone on the polymer team a while ago, and they said that you kinda end up with this. One big top level item, and then some smaller elements elsewhere.

Also, we have enough data being passed around that we need to link it together, and som-app is the main way we do that. 

:/
That's a shame that there's not really a way around the "big centralized file" pattern. I guess at the very least, we could try to refactor som-app so that it's duties are purely data passing and anything that's not strictly data passing is done by other elements. 

Or maybe we could still add some level of modularization? Like if we can find a set of elements that don't depend on anything outside of each other, they could be tied together by their own mini data passer which som-app then includes into the page. 
Project Member

Comment 3 by bugdroid1@chromium.org, Jan 13 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/infra/infra.git/+/2dca9340693c480b6a74d8170bec564764482a00

commit 2dca9340693c480b6a74d8170bec564764482a00
Author: Tiff Zhang <zhangtiff@google.com>
Date: Fri Jan 13 19:33:58 2017

SoM: Move elements into their own subdirectories.

BUG= 674660 

Change-Id: I833b24d75908422781683c9a8b02b624983bb1e6
Reviewed-on: https://chromium-review.googlesource.com/427414
Reviewed-by: Sean McCullough <seanmccullough@chromium.org>
Commit-Queue: Tiffany Zhang <zhangtiff@chromium.org>

[modify] https://crrev.com/2dca9340693c480b6a74d8170bec564764482a00/go/src/infra/appengine/sheriff-o-matic/.gitignore
[modify] https://crrev.com/2dca9340693c480b6a74d8170bec564764482a00/go/src/infra/appengine/sheriff-o-matic/Makefile
[delete] https://crrev.com/53c9b5d817c69198afa269c15bfbd8c196eb7eca/go/src/infra/appengine/sheriff-o-matic/elements/som-alert-item.html
[add] https://crrev.com/2dca9340693c480b6a74d8170bec564764482a00/go/src/infra/appengine/sheriff-o-matic/elements/som-alert-item/som-alert-item.html
[add] https://crrev.com/2dca9340693c480b6a74d8170bec564764482a00/go/src/infra/appengine/sheriff-o-matic/elements/som-alert-item/som-alert-item.js
[delete] https://crrev.com/53c9b5d817c69198afa269c15bfbd8c196eb7eca/go/src/infra/appengine/sheriff-o-matic/elements/som-annotations.html
[add] https://crrev.com/2dca9340693c480b6a74d8170bec564764482a00/go/src/infra/appengine/sheriff-o-matic/elements/som-annotations/som-annotations.html
[add] https://crrev.com/2dca9340693c480b6a74d8170bec564764482a00/go/src/infra/appengine/sheriff-o-matic/elements/som-annotations/som-annotations.js
[delete] https://crrev.com/53c9b5d817c69198afa269c15bfbd8c196eb7eca/go/src/infra/appengine/sheriff-o-matic/elements/som-app.html
[add] https://crrev.com/2dca9340693c480b6a74d8170bec564764482a00/go/src/infra/appengine/sheriff-o-matic/elements/som-app/som-app.html
[add] https://crrev.com/2dca9340693c480b6a74d8170bec564764482a00/go/src/infra/appengine/sheriff-o-matic/elements/som-app/som-app.js
[delete] https://crrev.com/53c9b5d817c69198afa269c15bfbd8c196eb7eca/go/src/infra/appengine/sheriff-o-matic/elements/som-bug-queue.html
[add] https://crrev.com/2dca9340693c480b6a74d8170bec564764482a00/go/src/infra/appengine/sheriff-o-matic/elements/som-bug-queue/som-bug-queue.html
[add] https://crrev.com/2dca9340693c480b6a74d8170bec564764482a00/go/src/infra/appengine/sheriff-o-matic/elements/som-bug-queue/som-bug-queue.js
[delete] https://crrev.com/53c9b5d817c69198afa269c15bfbd8c196eb7eca/go/src/infra/appengine/sheriff-o-matic/elements/som-drawer.html
[add] https://crrev.com/2dca9340693c480b6a74d8170bec564764482a00/go/src/infra/appengine/sheriff-o-matic/elements/som-drawer/som-drawer.html
[add] https://crrev.com/2dca9340693c480b6a74d8170bec564764482a00/go/src/infra/appengine/sheriff-o-matic/elements/som-drawer/som-drawer.js
[delete] https://crrev.com/53c9b5d817c69198afa269c15bfbd8c196eb7eca/go/src/infra/appengine/sheriff-o-matic/elements/som-examine.html
[add] https://crrev.com/2dca9340693c480b6a74d8170bec564764482a00/go/src/infra/appengine/sheriff-o-matic/elements/som-examine/som-examine.html
[add] https://crrev.com/2dca9340693c480b6a74d8170bec564764482a00/go/src/infra/appengine/sheriff-o-matic/elements/som-examine/som-examine.js
[delete] https://crrev.com/53c9b5d817c69198afa269c15bfbd8c196eb7eca/go/src/infra/appengine/sheriff-o-matic/elements/som-extension-build-failure.html
[add] https://crrev.com/2dca9340693c480b6a74d8170bec564764482a00/go/src/infra/appengine/sheriff-o-matic/elements/som-extension-build-failure/som-extension-build-failure.html
[add] https://crrev.com/2dca9340693c480b6a74d8170bec564764482a00/go/src/infra/appengine/sheriff-o-matic/elements/som-extension-build-failure/som-extension-build-failure.js
[delete] https://crrev.com/53c9b5d817c69198afa269c15bfbd8c196eb7eca/go/src/infra/appengine/sheriff-o-matic/elements/som-master-restarts.html
[add] https://crrev.com/2dca9340693c480b6a74d8170bec564764482a00/go/src/infra/appengine/sheriff-o-matic/elements/som-master-restarts/som-master-restarts.html
[add] https://crrev.com/2dca9340693c480b6a74d8170bec564764482a00/go/src/infra/appengine/sheriff-o-matic/elements/som-master-restarts/som-master-restarts.js
[delete] https://crrev.com/53c9b5d817c69198afa269c15bfbd8c196eb7eca/go/src/infra/appengine/sheriff-o-matic/elements/som-settings.html
[add] https://crrev.com/2dca9340693c480b6a74d8170bec564764482a00/go/src/infra/appengine/sheriff-o-matic/elements/som-settings/som-settings.html
[add] https://crrev.com/2dca9340693c480b6a74d8170bec564764482a00/go/src/infra/appengine/sheriff-o-matic/elements/som-settings/som-settings.js
[rename] https://crrev.com/2dca9340693c480b6a74d8170bec564764482a00/go/src/infra/appengine/sheriff-o-matic/elements/som-swarming-bots/som-swarming-bots.html
[add] https://crrev.com/2dca9340693c480b6a74d8170bec564764482a00/go/src/infra/appengine/sheriff-o-matic/elements/som-swarming-bots/som-swarming-bots.js
[delete] https://crrev.com/53c9b5d817c69198afa269c15bfbd8c196eb7eca/go/src/infra/appengine/sheriff-o-matic/elements/som-tree-status.html
[add] https://crrev.com/2dca9340693c480b6a74d8170bec564764482a00/go/src/infra/appengine/sheriff-o-matic/elements/som-tree-status/som-tree-status.html
[add] https://crrev.com/2dca9340693c480b6a74d8170bec564764482a00/go/src/infra/appengine/sheriff-o-matic/elements/som-tree-status/som-tree-status.js
[delete] https://crrev.com/53c9b5d817c69198afa269c15bfbd8c196eb7eca/go/src/infra/appengine/sheriff-o-matic/elements/som-webkit-tests.html
[add] https://crrev.com/2dca9340693c480b6a74d8170bec564764482a00/go/src/infra/appengine/sheriff-o-matic/elements/som-webkit-tests/som-webkit-tests.html
[add] https://crrev.com/2dca9340693c480b6a74d8170bec564764482a00/go/src/infra/appengine/sheriff-o-matic/elements/som-webkit-tests/som-webkit-tests.js
[modify] https://crrev.com/2dca9340693c480b6a74d8170bec564764482a00/go/src/infra/appengine/sheriff-o-matic/som/index.html
[modify] https://crrev.com/2dca9340693c480b6a74d8170bec564764482a00/go/src/infra/appengine/sheriff-o-matic/test/som-alert-item-test.html
[modify] https://crrev.com/2dca9340693c480b6a74d8170bec564764482a00/go/src/infra/appengine/sheriff-o-matic/test/som-annotations-test.html
[modify] https://crrev.com/2dca9340693c480b6a74d8170bec564764482a00/go/src/infra/appengine/sheriff-o-matic/test/som-app-test.html
[modify] https://crrev.com/2dca9340693c480b6a74d8170bec564764482a00/go/src/infra/appengine/sheriff-o-matic/test/som-bug-queue-test.html
[modify] https://crrev.com/2dca9340693c480b6a74d8170bec564764482a00/go/src/infra/appengine/sheriff-o-matic/test/som-drawer-test.html
[modify] https://crrev.com/2dca9340693c480b6a74d8170bec564764482a00/go/src/infra/appengine/sheriff-o-matic/test/som-examine-test.html
[modify] https://crrev.com/2dca9340693c480b6a74d8170bec564764482a00/go/src/infra/appengine/sheriff-o-matic/test/som-extension-build-failure-test.html
[modify] https://crrev.com/2dca9340693c480b6a74d8170bec564764482a00/go/src/infra/appengine/sheriff-o-matic/test/som-master-restarts-test.html
[modify] https://crrev.com/2dca9340693c480b6a74d8170bec564764482a00/go/src/infra/appengine/sheriff-o-matic/test/som-rev-range-test.html
[modify] https://crrev.com/2dca9340693c480b6a74d8170bec564764482a00/go/src/infra/appengine/sheriff-o-matic/test/som-swarming-bots-test.html
[modify] https://crrev.com/2dca9340693c480b6a74d8170bec564764482a00/go/src/infra/appengine/sheriff-o-matic/test/som-tree-status-test.html
[modify] https://crrev.com/2dca9340693c480b6a74d8170bec564764482a00/go/src/infra/appengine/sheriff-o-matic/test/som-webkit-tests-test.html

Status: Fixed (was: Untriaged)
I think the refactor to split js from the elements fixed this. I guess if we still think som-app is too big, we could always split the CSS as well, but at this point, I don't feel like this is necessary. 

Sign in to add a comment