Bug 1640760 Comment 35 Edit History

Note: The actual edited comment in the bug view page will always show the original commenter’s name and original timestamp.

(In reply to Alessandro Castellani (:aleca) from comment #27)
> > I have a big question for this patch: Can we please stick to agreed procedures and avoid combining a massive code cleanup/rewrite (without functional changes) with new feature/UI work?
> 
> I'll let Magnus decide on this, no problem on my end in doing the code clean up in a separated bug prior to this.

Hey Alessandro, thank you for indicating that it's no problem for you to split out the code cleanup and indenting changes into a separate patch (and yeah, ideally on a separate bug). I know it's boring, but I think that would make reviewer's life here (including my feedback which you've asked for) a LOT easier because with the current patch, it's all but impossible to identify the relevant changes of the actual UI refactoring between loads of unrelated little rewrites and indenting changes. It's easy to overlook flaws in this combined patch. It will also save us from a lot of trouble if either the cleanup or the refactoring happen to break something - if we land them separately, it will be much easier to identify the culprit and to avoid searching the needle in the haystack. I guess there's a reason for one-issue-per-bug, and having a "task" category for code cleanup bugs. Could you do that? Pretty-please... Tia!!!
(In reply to Alessandro Castellani (:aleca) from comment #27)
> > I have a big question for this patch: Can we please stick to agreed procedures and avoid combining a massive code cleanup/rewrite (without functional changes) with new feature/UI work?
> 
> I'll let Magnus decide on this, no problem on my end in doing the code clean up in a separated bug prior to this.

Hey Alessandro, thank you for indicating that it's no problem for you to split out the code cleanup and indenting changes into a separate patch (and yeah, ideally on a separate bug). I know it's boring, but I think that would make reviewer's life here (including my feedback which you've asked for) a LOT easier because with the current patch, it's all but impossible to identify the relevant changes of the actual UI refactoring between loads of unrelated little rewrites and indenting changes. It's easy to overlook flaws in this combined patch. It will also save us from a lot of trouble if either the cleanup or the refactoring happen to break something - if we land them separately, it will be much easier to identify the culprit and to avoid searching the needle in the haystack. I guess there's a reason for one-issue-per-bug, and having a "task" flavor for code cleanup bugs. Could you do that? Pretty-please... Tia!!!

Back to Bug 1640760 Comment 35