Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code

Issue 29362580: Issue 4641 - Append performance fix to trac cgi file (Closed)

Created:
Nov. 15, 2016, 6:42 p.m. by f.nicolaisen
Modified:
Nov. 21, 2016, 7:20 a.m.
CC:
f.lopez
Visibility:
Public.

Description

We’re not moving trac yet. This change improves trac setup fully provisioned by puppet.

Patch Set 1 #

Total comments: 7

Patch Set 2 : Issue 4641 - Performance fix to trac cgi file #

Total comments: 13

Patch Set 3 : Issue 4641 - Performance fix to trac cgi file #

Patch Set 4 : Issue 4641 - Performance fix to trac cgi file #

Total comments: 6

Patch Set 5 : Drop arrow aligning, use root, keep under 80 cols #

Patch Set 6 : Forgot the py file #

Unified diffs Side-by-side diffs Delta from patch set Stats (+27 lines, -1 line) Patch
A modules/trac/files/trac_performance_fix.py View 1 2 5 1 chunk +8 lines, -0 lines 0 comments Download
M modules/trac/manifests/init.pp View 1 2 3 4 3 chunks +19 lines, -1 line 0 comments Download

Messages

Total messages: 15
Wladimir Palant
If you don't send mail, nobody will notice that a review is created (hg review ...
Nov. 16, 2016, 9:26 a.m. (2016-11-16 09:26:18 UTC) #1
f.nicolaisen
Thanks for the feedback and the directions! One point is still unclear to me (see ...
Nov. 16, 2016, 3:01 p.m. (2016-11-16 15:01:28 UTC) #2
Wladimir Palant
https://codereview.adblockplus.org/29362580/diff/29362581/modules/trac/manifests/init.pp File modules/trac/manifests/init.pp (right): https://codereview.adblockplus.org/29362580/diff/29362581/modules/trac/manifests/init.pp#newcode264 modules/trac/manifests/init.pp:264: path => '/usr/bin:/usr/sbin:/bin:/usr/local/bin', On 2016/11/16 15:01:28, f.nicolaisen wrote: > ...
Nov. 16, 2016, 3:16 p.m. (2016-11-16 15:16:44 UTC) #3
f.nicolaisen
On 2016/11/16 15:16:44, Wladimir Palant wrote: > https://codereview.adblockplus.org/29362580/diff/29362581/modules/trac/manifests/init.pp > File modules/trac/manifests/init.pp (right): > > https://codereview.adblockplus.org/29362580/diff/29362581/modules/trac/manifests/init.pp#newcode264 ...
Nov. 16, 2016, 4:57 p.m. (2016-11-16 16:57:52 UTC) #4
Wladimir Palant
https://codereview.adblockplus.org/29362580/diff/29362602/modules/trac/manifests/init.pp File modules/trac/manifests/init.pp (right): https://codereview.adblockplus.org/29362580/diff/29362602/modules/trac/manifests/init.pp#newcode261 modules/trac/manifests/init.pp:261: require => File["/home/trac/htdocs-$name/cgi-bin/trac_performance_fix.snippet"], This should also require Exec["deploy_$name"], it ...
Nov. 16, 2016, 5:02 p.m. (2016-11-16 17:02:52 UTC) #5
f.nicolaisen
https://codereview.adblockplus.org/29362580/diff/29362602/modules/trac/manifests/init.pp File modules/trac/manifests/init.pp (right): https://codereview.adblockplus.org/29362580/diff/29362602/modules/trac/manifests/init.pp#newcode261 modules/trac/manifests/init.pp:261: require => File["/home/trac/htdocs-$name/cgi-bin/trac_performance_fix.snippet"], On 2016/11/16 17:02:52, Wladimir Palant wrote: ...
Nov. 16, 2016, 5:04 p.m. (2016-11-16 17:04:55 UTC) #6
Wladimir Palant
LGTM https://codereview.adblockplus.org/29362580/diff/29362602/modules/trac/manifests/init.pp File modules/trac/manifests/init.pp (right): https://codereview.adblockplus.org/29362580/diff/29362602/modules/trac/manifests/init.pp#newcode261 modules/trac/manifests/init.pp:261: require => File["/home/trac/htdocs-$name/cgi-bin/trac_performance_fix.snippet"], On 2016/11/16 17:04:55, f.nicolaisen wrote: ...
Nov. 16, 2016, 6:57 p.m. (2016-11-16 18:57:33 UTC) #7
mathias
https://codereview.adblockplus.org/29362580/diff/29362602/modules/trac/files/trac_performance_fix.snippet File modules/trac/files/trac_performance_fix.snippet (right): https://codereview.adblockplus.org/29362580/diff/29362602/modules/trac/files/trac_performance_fix.snippet#newcode4 modules/trac/files/trac_performance_fix.snippet:4: def patched_get(self, key, default=none): Typo: "None" vs. "none". Also, ...
Nov. 17, 2016, 7:43 a.m. (2016-11-17 07:43:32 UTC) #8
f.nicolaisen
https://codereview.adblockplus.org/29362580/diff/29362602/modules/trac/files/trac_performance_fix.snippet File modules/trac/files/trac_performance_fix.snippet (right): https://codereview.adblockplus.org/29362580/diff/29362602/modules/trac/files/trac_performance_fix.snippet#newcode4 modules/trac/files/trac_performance_fix.snippet:4: def patched_get(self, key, default=none): On 2016/11/17 07:43:31, mathias wrote: ...
Nov. 17, 2016, 8:23 a.m. (2016-11-17 08:23:46 UTC) #9
mathias
https://codereview.adblockplus.org/29362580/diff/29362602/modules/trac/files/trac_performance_fix.snippet File modules/trac/files/trac_performance_fix.snippet (right): https://codereview.adblockplus.org/29362580/diff/29362602/modules/trac/files/trac_performance_fix.snippet#newcode4 modules/trac/files/trac_performance_fix.snippet:4: def patched_get(self, key, default=none): On 2016/11/17 08:23:46, f.nicolaisen wrote: ...
Nov. 17, 2016, 10:18 a.m. (2016-11-17 10:18:33 UTC) #10
f.nicolaisen
https://codereview.adblockplus.org/29362580/diff/29362602/modules/trac/files/trac_performance_fix.snippet File modules/trac/files/trac_performance_fix.snippet (right): https://codereview.adblockplus.org/29362580/diff/29362602/modules/trac/files/trac_performance_fix.snippet#newcode4 modules/trac/files/trac_performance_fix.snippet:4: def patched_get(self, key, default=none): On 2016/11/17 10:18:33, mathias wrote: ...
Nov. 17, 2016, 10:54 a.m. (2016-11-17 10:54:32 UTC) #11
f.nicolaisen
New strategy: use file_line to inject a python import statement, as suggested by Matze in ...
Nov. 17, 2016, 9:48 p.m. (2016-11-17 21:48:09 UTC) #12
mathias
Almost there ;-) https://codereview.adblockplus.org/29362580/diff/29363527/modules/trac/manifests/init.pp File modules/trac/manifests/init.pp (right): https://codereview.adblockplus.org/29362580/diff/29363527/modules/trac/manifests/init.pp#newcode152 modules/trac/manifests/init.pp:152: ensure => present, Please do not ...
Nov. 17, 2016, 10:08 p.m. (2016-11-17 22:08:37 UTC) #13
f.nicolaisen
https://codereview.adblockplus.org/29362580/diff/29363527/modules/trac/manifests/init.pp File modules/trac/manifests/init.pp (right): https://codereview.adblockplus.org/29362580/diff/29363527/modules/trac/manifests/init.pp#newcode152 modules/trac/manifests/init.pp:152: ensure => present, On 2016/11/17 22:08:37, mathias wrote: > ...
Nov. 18, 2016, 9:23 a.m. (2016-11-18 09:23:30 UTC) #14
mathias
Nov. 18, 2016, 9:35 a.m. (2016-11-18 09:35:28 UTC) #15
LGTM.

Powered by Google App Engine
This is Rietveld