Le mercredi 22 novembre 2017 à 13:33:30+0800, Paul Wise a écrit :
> On Wed, 2017-11-22 at 00:00 +0100, Pierre-Elliott Bécue wrote:
>
> > As for an example, you can have a look on my test tracker:
>
> Great work!
>
> > https://distro-tracker.pimeys.fr/pkg/kholidays
> > https://distro-tracker.pimeys.fr/pkg/puppet-module-puppetlabs-rsync
>
> Minor nitpick, these include punctuation inside the links, I would
> suggest moving that outside the link.
I'll have a look about it.
> > And one working well:
> > https://distro-tracker.pimeys.fr/pkg/python-aiosmtpd
>
> This seems to be missing the QA link, but the patch you provided
> doesn't seem to conditionalise on the status and the package is
> definitely present within the vcswatch data.
I only show QA link when status is not OK. So no QA there. Would you like a
QA link for each package?
> > Please, feel free to review and comment the patch.
>
> Great, some minor comments below.
>
> > Subject: [PATCH 1/3] Adds compression utilities for future use with caches
>
> Would it be a good idea to add xz in addition to or instead of bzip2?
Sure!
> I'm surprised there isn't a standard implementation of this.
I didn't find anything relevant so far.
> > + compressed_magic_bits_map = {
> > + b"\x1f\x8b\x08": "gzip",
> > + b"\x42\x5a\x68": "bz2",
>
> I'm not sure it is a good idea to embed these.
>
> Also, the magic numbers should both be two bytes, not three:
>
> https://en.wikipedia.org/wiki/Gzip#File_format
Yeah, I read otherwise somewhere else, but it seems that I read wrong.
> https://en.wikipedia.org/wiki/BZ2#File_format
Bzip2 needs two magic bytes and a version byte, hence the three bytes.
In any case, you're right, embedding this is not that much relevant, let's
rely on file extensions.
>
> > + def get_content(self, url, compression=False):
> > +def get_resource_content(url, cache=None, compression=False):
> > + `url`. If False, then no compression. Otherwise, uses the
>
> I would use compression=None here. I also wonder if the parameter is
> needed, it could just transparently decompress when needed, then the
> callers would not need to do anything special for compressed URLs.
I started with a silent "decompression wizard" and then thought it'd be
better to have explicit intel for the user.
> > Subject: [PATCH 3/3] Implements VCSWatch in the tracker.
> > +<a href="{{item.extra_data.vcswatch_url}}">VCSwatch</a> reports
>
> The vcswatch service refers to itself in lower case :)
>
> https://qa.debian.org/cgi-bin/vcswatch
Ok.
> > + "VCS. You should consider updating the debian/changelog "
> > + "and to upload this new version into the archive."
>
> s/to upload/uploading/
>
> > + "the current version of the package is NOT in its "
> > + "VCS. You should upload your changes immediately."
>
> s/upload your changes/push your commits/
>
> > + "this package has been uploaded into the archive but "
> > + "the debian/changelog into the VCS is still UNRELEASED. "
>
> s/into/in/
Ok
> > + "you shouldn't see this report. Please report this bug "
> > + "to the tracker's maintainers with the current URL of "
> > + "the page you're seeing."
>
> The first part doesn't really fit with the sentence format :)
>
> VCSwatch reports that you shouldn't see this report.
>
> Maybe something like this:
>
> vcswatch reports that a new type of VCS status has been added.
> Please report a bug about this so that the maintainers can describe it.
>
> With report linked to a mailto for the bug tracker (properly encoded):
>
> mailto:
> submit@bugs.debian.org
>
> subject
> tracker.debian.org: $pkgname: new vcswatch status: $statusname
>
> body
> Package: tracker.debian.org
> User: tracker.debian.org@packages.debian.org
> Usertags: vcswatch
>
> The vcswatch status for $pkgname is $statusname and
> this is not known by the tracker website:
>
> https://tracker.debian.org/pkg/$pkgname
I'll look into it.
--
PEB
Attachment:
signature.asc
Description: PGP signature