Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(4777)

Issue 10756043: Update makefile template and css build targets.

Can't Edit
Can't Publish+Mail
Start Review
Created:
10 years, 9 months ago by rharding
Modified:
10 years, 9 months ago
Reviewers:
mp+172117, j.c.sackett, benji
Visibility:
Public.

Description

Update makefile template and css build targets. - The template targets didn't take into account the subapps directory. - There was no direct target watching css files. Since they're built as part of the templates I think it's typical for a template change to accompany css changes and it wasn't noticed there was a corner case. QA -- To QA you need to run `make build-prod`. Once that is done, run it again and see that it does not rerun any builders on the template/css targets. Then, touch a template in the subapps directory and make build-prod again. `touch app/subapps/browser/templates/minimized.handlebars && make build-prod` Run build-prod again and see that there's no change. Then touch a less file and verify the makefile catches it as changed and rebuilds. `touch lib/views/browser/charm-token.less && make build-prod` https://code.launchpad.net/~rharding/juju-gui/makefile-updates/+merge/172117 (do not edit description out of merge proposal)

Patch Set 1 #

Total comments: 1

Patch Set 2 : Update makefile template and css build targets. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+7 lines, -2 lines) Patch
M Makefile View 1 2 chunks +5 lines, -2 lines 0 comments Download
A [revision details] View 1 1 chunk +2 lines, -0 lines 0 comments Download

Messages

Total messages: 4
rharding
Please take a look.
10 years, 9 months ago (2013-06-28 18:03:11 UTC) #1
j.c.sackett
On 2013/06/28 18:03:11, rharding wrote: > Please take a look. LGTM.
10 years, 9 months ago (2013-06-28 18:16:18 UTC) #2
benji
Nice. I had one small regex suggestion. LGTM-ly yrs, Benji https://codereview.appspot.com/10756043/diff/1/Makefile File Makefile (right): https://codereview.appspot.com/10756043/diff/1/Makefile#newcode125 ...
10 years, 9 months ago (2013-06-28 18:29:11 UTC) #3
rharding
10 years, 9 months ago (2013-06-28 18:36:32 UTC) #4
*** Submitted:

Update makefile template and css build targets.

- The template targets didn't take into account the subapps directory.
- There was no direct target watching css files. Since they're built as part
of the templates I think it's typical for a template change to accompany css
changes and it wasn't noticed there was a corner case.

QA
--

To QA you need to run `make build-prod`. Once that is done, run it again and
see that it does not rerun any builders on the template/css targets.

Then, touch a template in the subapps directory and make build-prod again.

`touch app/subapps/browser/templates/minimized.handlebars && make build-prod`

Run build-prod again and see that there's no change. Then touch a less file
and verify the makefile catches it as changed and rebuilds.

`touch lib/views/browser/charm-token.less && make build-prod`

R=j.c.sackett, benji
CC=
https://codereview.appspot.com/10756043
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b