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

Issue 29323409: Issue 2867 - Introduce module hgweb and corresponding server role (Closed)

Created:
Aug. 10, 2015, 8:15 a.m. by mathias
Modified:
Aug. 24, 2015, 5:38 p.m.
CC:
Fred
Visibility:
Public.

Description

Issue 2867 - Introduce module hgweb and corresponding server role

Patch Set 1 #

Patch Set 2 : Issue 2867 - Fixes for issues discovered during test + merge with upstream #

Total comments: 5

Patch Set 3 : Issue 2867 - Remove double blank lines #

Patch Set 4 : Issue 2867 - Now the complete patch-set again #

Total comments: 6
Unified diffs Side-by-side diffs Delta from patch set Stats (+378 lines, -1 line) Patch
A hiera/roles/hgwebserver.yaml View 3 1 chunk +5 lines, -0 lines 0 comments Download
A modules/hgweb/files/hgaccess/repos/hgaccess View 3 1 chunk +1 line, -0 lines 0 comments Download
A modules/hgweb/files/hgaccess/users/vagrant View 3 1 chunk +1 line, -0 lines 0 comments Download
A modules/hgweb/files/hgweb.fcgi View 1 3 1 chunk +95 lines, -0 lines 0 comments Download
A modules/hgweb/files/hgweb.ini View 3 1 chunk +14 lines, -0 lines 0 comments Download
A modules/hgweb/files/hgweb.sh View 3 1 chunk +37 lines, -0 lines 0 comments Download
A modules/hgweb/files/nginx.conf View 3 1 chunk +17 lines, -0 lines 6 comments Download
A modules/hgweb/files/robots.txt View 3 1 chunk +2 lines, -0 lines 0 comments Download
A modules/hgweb/manifests/init.pp View 1 2 3 1 chunk +180 lines, -0 lines 0 comments Download
A modules/hgweb/templates/hgrc.erb View 3 1 chunk +11 lines, -0 lines 0 comments Download
A modules/hgweb/templates/sitescripts.ini.erb View 1 3 1 chunk +9 lines, -0 lines 0 comments Download
M modules/nginx/manifests/params.pp View 1 2 3 1 chunk +1 line, -1 line 0 comments Download
M modules/private-stub/hiera/hosts.yaml View 3 1 chunk +5 lines, -0 lines 0 comments Download

Messages

Total messages: 11
mathias
Aug. 10, 2015, 8:15 a.m. (2015-08-10 08:15:58 UTC) #1
mathias
Aug. 17, 2015, 10:50 a.m. (2015-08-17 10:50:54 UTC) #2
Felix Dahlke
Looks pretty good as far as I'm concerned, just a few smaller comments. Plus, I'm ...
Aug. 17, 2015, 6:05 p.m. (2015-08-17 18:05:04 UTC) #3
mathias
Aug. 17, 2015, 6:14 p.m. (2015-08-17 18:14:39 UTC) #4
mathias
Well, I don't really care about the naming that much. But if I where to ...
Aug. 17, 2015, 6:16 p.m. (2015-08-17 18:16:45 UTC) #5
mathias
Aug. 17, 2015, 6:17 p.m. (2015-08-17 18:17:53 UTC) #6
Felix Dahlke
Wouldn't insist on a different name, hgweb is fine by me. https://codereview.adblockplus.org/29323409/diff/29323754/modules/hgweb/files/hgaccess/repos/hgaccess File modules/hgweb/files/hgaccess/repos/hgaccess (right): ...
Aug. 17, 2015, 6:21 p.m. (2015-08-17 18:21:20 UTC) #7
mathias
On 2015/08/17 18:21:20, Felix Dahlke wrote: > Seems pretty inconsistent with how we usually do ...
Aug. 17, 2015, 6:28 p.m. (2015-08-17 18:28:09 UTC) #8
Felix Dahlke
On 2015/08/17 18:28:09, mathias wrote: > On 2015/08/17 18:21:20, Felix Dahlke wrote: > > Seems ...
Aug. 17, 2015, 6:43 p.m. (2015-08-17 18:43:07 UTC) #9
Wladimir Palant
https://codereview.adblockplus.org/29323409/diff/29323871/modules/hgweb/files/nginx.conf File modules/hgweb/files/nginx.conf (right): https://codereview.adblockplus.org/29323409/diff/29323871/modules/hgweb/files/nginx.conf#newcode1 modules/hgweb/files/nginx.conf:1: add_header Strict-Transport-Security "max-age=2592000"; This is duplicating String-Transport-Security header from ...
Aug. 24, 2015, 4:49 p.m. (2015-08-24 16:49:49 UTC) #10
mathias
Aug. 24, 2015, 5:38 p.m. (2015-08-24 17:38:46 UTC) #11
Message was sent while issue was closed.
See https://codereview.adblockplus.org/29324553/ --

https://codereview.adblockplus.org/29323409/diff/29323871/modules/hgweb/files...
File modules/hgweb/files/nginx.conf (right):

https://codereview.adblockplus.org/29323409/diff/29323871/modules/hgweb/files...
modules/hgweb/files/nginx.conf:1: add_header Strict-Transport-Security
"max-age=2592000";
On 2015/08/24 16:49:48, Wladimir Palant wrote:
> This is duplicating String-Transport-Security header from the generic
> configuration - this server is currently sending two String-Transport-Security
> headers.

Done.

https://codereview.adblockplus.org/29323409/diff/29323871/modules/hgweb/files...
modules/hgweb/files/nginx.conf:3: location / {
On 2015/08/24 16:49:48, Wladimir Palant wrote:
> We generally put opening brackets on the next line in nginx configuration.

Done.

https://codereview.adblockplus.org/29323409/diff/29323871/modules/hgweb/files...
modules/hgweb/files/nginx.conf:12: root /usr/share/mercurial/templates;
On 2015/08/24 16:49:48, Wladimir Palant wrote:
> This isn't pointing to the Mercurial instance running on the server right now,
> correct path would be
> /usr/local/lib/python2.7/dist-packages/mercurial/templates/.

Done.

Powered by Google App Engine
This is Rietveld