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

Issue 29481626: #1980 - Introduce class adblockplus::web (Closed)

Created:
July 6, 2017, 11:49 a.m. by mathias
Modified:
July 6, 2017, 1:10 p.m.
Reviewers:
Fred
CC:
f.lopez, f.nicolaisen
Visibility:
Public.

Description

Inclusion with class filterserver only for illustration purpose, will become a separate commit

Patch Set 1 #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+27 lines, -9 lines) Patch
A modules/adblockplus/manifests/web.pp View 1 chunk +26 lines, -0 lines 2 comments Download
M modules/filterserver/manifests/init.pp View 1 chunk +1 line, -9 lines 0 comments Download

Messages

Total messages: 4
mathias
July 6, 2017, 11:49 a.m. (2017-07-06 11:49:36 UTC) #1
Fred
https://codereview.adblockplus.org/29481626/diff/29481627/modules/adblockplus/manifests/web.pp File modules/adblockplus/manifests/web.pp (right): https://codereview.adblockplus.org/29481626/diff/29481627/modules/adblockplus/manifests/web.pp#newcode21 modules/adblockplus/manifests/web.pp:21: 'mode' => '0755', Shouldn't also a user (owner) be ...
July 6, 2017, 12:33 p.m. (2017-07-06 12:33:00 UTC) #2
mathias
https://codereview.adblockplus.org/29481626/diff/29481627/modules/adblockplus/manifests/web.pp File modules/adblockplus/manifests/web.pp (right): https://codereview.adblockplus.org/29481626/diff/29481627/modules/adblockplus/manifests/web.pp#newcode21 modules/adblockplus/manifests/web.pp:21: 'mode' => '0755', On 2017/07/06 12:33:00, Fred wrote: > ...
July 6, 2017, 12:42 p.m. (2017-07-06 12:42:06 UTC) #3
Fred
July 6, 2017, 12:45 p.m. (2017-07-06 12:45:18 UTC) #4
On 2017/07/06 12:42:06, mathias wrote:
>
https://codereview.adblockplus.org/29481626/diff/29481627/modules/adblockplus...
> File modules/adblockplus/manifests/web.pp (right):
> 
>
https://codereview.adblockplus.org/29481626/diff/29481627/modules/adblockplus...
> modules/adblockplus/manifests/web.pp:21: 'mode' => '0755',
> On 2017/07/06 12:33:00, Fred wrote:
> > Shouldn't also a user (owner) be provided as a default?
> > Although that might depend on the actual webserver package in use.
> 
> In the current state this defaults to the user executing Puppet, which is
always
> root in our case. And no, the HTTPd package does not create that directory.

Well, root should be OK as default. So
LGTM

Powered by Google App Engine
This is Rietveld