launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging ~cjwatson/launchpad:combinecss-bytes into launchpad:master.

Commit message:
Make combinecss.main deal entirely in bytes

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/377339

Also explain why we're dealing in bytes rather than text here (due to the interaction with cssutils), since it isn't obvious.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:combinecss-bytes into launchpad:master.

diff --git a/lib/lp/scripts/utilities/js/combinecss.py b/lib/lp/scripts/utilities/js/combinecss.py
index bf52ed1..885d11c 100755
--- a/lib/lp/scripts/utilities/js/combinecss.py
+++ b/lib/lp/scripts/utilities/js/combinecss.py
@@ -75,9 +75,9 @@ def main():
 
     combo = ComboFile(absolute_names, target)
     if combo.needs_update():
-        result = u''
+        result = b''
         for content in combine_files(names, icing):
-            result += content.decode('utf8')
+            result += content
 
         with open(target, 'wb') as f:
-            f.write(result.encode('utf8'))
+            f.write(result)
diff --git a/lib/lp/scripts/utilities/js/combo.py b/lib/lp/scripts/utilities/js/combo.py
index f568bc2..b02e381 100644
--- a/lib/lp/scripts/utilities/js/combo.py
+++ b/lib/lp/scripts/utilities/js/combo.py
@@ -41,6 +41,10 @@ def combine_files(fnames, root, resource_prefix=b"",
     Returns an iterator with the combined content of all the
     files. The relative path to root will be included as a comment
     between each file.
+
+    Although CSS files are conceptually closer to text than bytes, we always
+    yield bytes here since that's closer to what cssutils gives us, and it
+    saves having to know the encoding.
     """
 
     combo_by_kind = {