Code review comment for lp:~sylvain-pineau/checkbox/xlsx_exporter_new_categories

Revision history for this message
Zygmunt Krynicki (zyga) wrote :

I'm pretty unhappy about landing that just like that.

- The commit message is lacking, you could have made separate commits for this
- The feature is questionable, it seems that we're *AGAIN* using internal identifiers for dislay where we *ALREADY* have better data. Why are we doing that?
- Some coding below is odd but that's a lesser thing

In general I blame the deadline for the demo on Monday. I really don't like patches that show up, self-approved, on Friday before a demo Monday. Sigh.

Please patch this.

review: Needs Fixing

« Back to merge proposal