Code review comment for lp:~danci-emanuel/mailman/dlists_support_added

Emanuel Danci (danci-emanuel) wrote :

> This all looks reasonable, but I don't have the larger context of what these
> files are actually doing.
> At line 114, why did you delete mlist.advertised?
> It also looks like you are arbitrarily changing the indentation at lines 44
> and 82. That's not good.
> With those changes, it's probably OK for now. (you need a larger context to be
> able to test it, I think -- like something that uses the dlists_enabled
> variable?

Indeed, I deleted mlist.advertised while doing some testing and I missed adding it back. Thanks for outlining this!
As for the indentation, I changed it while adding some code in those parts, but although I changed it back to the original form, it seems that the commit did not take into considerations those changes. In those parts, the code looks like in the original files now.

« Back to merge proposal