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

Issue 29586667: Make mimeo module only listen to localhost (Closed)

Created:
Oct. 23, 2017, 3:41 p.m. by f.lopez
Modified:
Oct. 24, 2017, 8:48 p.m.
CC:
f.nicolaisen
Visibility:
Public.

Description

Make mimeo module only listen to localhost

Patch Set 1 #

Patch Set 2 : Use appropiate parameter to manage listening address instead of hardcoded one #

Patch Set 3 : Fixing typo #

Unified diffs Side-by-side diffs Delta from patch set Stats (+5 lines, -1 line) Patch
M modules/adblockplus/files/mimeo.py View 1 2 2 chunks +5 lines, -1 line 0 comments Download

Messages

Total messages: 7
f.lopez
Oct. 23, 2017, 3:41 p.m. (2017-10-23 15:41:05 UTC) #1
f.nicolaisen
Please point out in the commit message that you're only changing the default configuration.
Oct. 23, 2017, 3:46 p.m. (2017-10-23 15:46:53 UTC) #2
rscott
LGTM
Oct. 23, 2017, 4:01 p.m. (2017-10-23 16:01:10 UTC) #3
Vasily Kuznetsov
LGTM on the code, but the commit message should be either 'Noissue - Introduce command ...
Oct. 24, 2017, 4:01 p.m. (2017-10-24 16:01:53 UTC) #4
mathias
On 2017/10/24 16:01:53, Vasily Kuznetsov wrote: > LGTM on the code, but the commit message ...
Oct. 24, 2017, 4:20 p.m. (2017-10-24 16:20:01 UTC) #5
Vasily Kuznetsov
On 2017/10/24 16:20:01, mathias wrote: > On 2017/10/24 16:01:53, Vasily Kuznetsov wrote: > > LGTM ...
Oct. 24, 2017, 4:24 p.m. (2017-10-24 16:24:05 UTC) #6
mathias
Oct. 24, 2017, 4:39 p.m. (2017-10-24 16:39:07 UTC) #7
On 2017/10/24 16:24:05, Vasily Kuznetsov wrote:
> Fair enough, but at least the commit message should mention both changes: (1)
> listen only on localhost and (2) introduce command line option.

Right, the commit message is not optimal. @paco What Vasily wrote. You can use
the classic do/what/where pattern: "Introduce --address parameter with mimeo.py
script". The default now being localhost is an implicit detail one can neglect
in the commit message, IMHO, irregardless whether it was the initial motivation.
But since there's no associated ticket for background information, associated
details like the motivation for this patch-set should end up in the extended
part of the commit message.

Powered by Google App Engine
This is Rietveld