launchpad-reviewers team mailing list archive

[William Grant <me@xxxxxxxxxxxxxxxxxx>]

Review: Approve code



Diff comments:

> === modified file 'lib/lp/archivepublisher/publishing.py'
> --- lib/lp/archivepublisher/publishing.py	2014-07-28 01:58:49 +0000
> +++ lib/lp/archivepublisher/publishing.py	2014-07-30 01:37:03 +0000
> @@ -900,8 +900,8 @@
>          self.log.debug("Writing Index file for %s/%s/i18n" % (
>              suite, component))
>  
> -        i18n_dir = os.path.join(self._config.distsroot, suite, component,
> -                                "i18n")
> +        i18n_subpath = os.path.join(component, "i18n")
> +        i18n_dir = os.path.join(self._config.distsroot, suite, i18n_subpath)
>          i18n_files = []
>          try:
>              for i18n_file in os.listdir(i18n_dir):
> @@ -923,13 +923,15 @@
>          i18n_index = I18nIndex()
>          for i18n_file in sorted(i18n_files):
>              entry = self._readIndexFileContents(
> -                suite, os.path.join(component, "i18n", i18n_file))
> +                suite, os.path.join(i18n_subpath, i18n_file))
>              if entry is None:
>                  continue
>              i18n_index.setdefault("SHA1", []).append({
>                  "sha1": hashlib.sha1(entry).hexdigest(),
>                  "name": i18n_file,
>                  "size": len(entry)})
> +            # Schedule i18n files for inclusion in the Release file.
> +            all_series_files.add(os.path.join(i18n_subpath, i18n_file))
>  
>          with open(os.path.join(i18n_dir, "Index"), "w") as f:
>              i18n_index.dump(f, "utf-8")
> 
> === modified file 'lib/lp/archivepublisher/tests/test_publisher.py'
> --- lib/lp/archivepublisher/tests/test_publisher.py	2014-07-28 01:58:49 +0000
> +++ lib/lp/archivepublisher/tests/test_publisher.py	2014-07-30 01:37:03 +0000
> @@ -1560,19 +1560,28 @@
>          publisher.C_doFTPArchive(False)
>          publisher.D_writeReleaseFiles(False)
>  
> -        i18n_index = os.path.join(
> -            self.config.distsroot, 'breezy-autotest', 'main', 'i18n', 'Index')
> +        series = os.path.join(self.config.distsroot, 'breezy-autotest')
> +        i18n_index = os.path.join(series, 'main', 'i18n', 'Index')
>  
>          # The i18n/Index file has been generated.
>          self.assertTrue(os.path.exists(i18n_index))
>  
>          # It is listed correctly in Release.
> -        release = self.parseRelease(os.path.join(
> -            self.config.distsroot, 'breezy-autotest', 'Release'))
> +        release = self.parseRelease(os.path.join(series, 'Release'))
>          with open(i18n_index) as i18n_index_file:
>              self.assertReleaseContentsMatch(
>                  release, 'main/i18n/Index', i18n_index_file.read())
>  
> +        files = ['main/i18n/Translation-en.bz2',
> +                 'universe/i18n/Translation-en.bz2',
> +                 'multiverse/i18n/Translation-en.bz2',
> +                 'multiverse/i18n/Translation-en.bz2']

All these paths are very similar. Why not just iterate over the component names?

> +        release_path = os.path.join(series, 'Release')
> +        with open(release_path) as release_file:
> +            content = release_file.read()
> +            for file in files:
> +                self.assertIn(file, content)
> +
>      def testCreateSeriesAliasesNoAlias(self):
>          """createSeriesAliases has nothing to do by default."""
>          publisher = Publisher(
> @@ -1683,10 +1692,14 @@
>          self.assertEqual(str(len(translation_en_contents)),
>                           i18n_index['sha1'][0]['size'])
>  
> -        # i18n/Index is scheduled for inclusion in Release.
> -        self.assertEqual(1, len(all_files))
> +        # i18n/Index and i18n/Translation-en.bz2 are scheduled for inclusion
> +        # in Release.
> +        self.assertEqual(2, len(all_files))
>          self.assertEqual(
>              os.path.join('main', 'i18n', 'Index'), all_files.pop())
> +        self.assertEqual(
> +            os.path.join('main', 'i18n', 'Translation-en.bz2'),
> +                         all_files.pop())
>  
>      def testWriteSuiteI18nMissingDirectory(self):
>          """i18n/Index is not generated when the i18n directory is missing."""
> 


-- 
https://code.launchpad.net/~cjohnston/launchpad/1313576-release-hashes/+merge/228744
Your team Launchpad code reviewers is subscribed to branch lp:launchpad.