Bug#758502: tracker.debian.org: Add data from duck.debian.net to "action needed" panel
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA512
Am 2014-08-19 11:13, schrieb Raphael Hertzog:
> Hello Simon,
>
> On Mon, 18 Aug 2014, Simon Kainz wrote:
>> this adds support for data from duck.debian.net.
>
> Thanks for the patch! Here are a few things to fix:
>
> First you should respect PEP8 for the coding style, notably: - use
> only spaces to indent, not tabs - spaces around assignation
> operatior
>
> You can use "pep8" to check your code (distro-tracker is not fully
> PEP8 compliant yet, but we try to pay attention for new code).
ok, i did and fixed the areas where it complains about code written by me
>
>>> From a0a3810608a306ab77eee214a9be0854e39e3367 Mon Sep 17
>>> 00:00:00 2001
>> From: Simon Kainz Date: Mon, 18 Aug 2014
>> 10:42:28 +0200 Subject: [PATCH 1/2] add new template for duck
>> extra info.
>
> There's no point in adding the template in a separate commit.
> Please squash both commits together.
ok, done, see the attachment.
>
>> --- /dev/null +++
>> b/distro_tracker/vendor/debian/templates/debian/duck-action-item.html
>>
>>
@@ -0,0 +1,8 @@
>> +{% with duck = item.extra_data.duck_link %} +{% with issues =
>> item.extra_data.issues_link %} +DUCK
>> reports some issues concerning
>> upstream URLs defined for this package. +{% endwith %} +{%
>> endwith %}
>
> The "with" tag doesn't bring anything as you use each variable only
> once. Please inline the duck and issues variables.
fixed.
>
>> From 11e3918f8f9b99b96e0d77c11ddf686dc36d20f1 Mon Sep 17 00:00:00
>> 2001 From: Simon Kainz Date: Mon, 18 Aug
>> 2014 10:43:14 +0200 Subject: [PATCH 2/2] add data from
>> duck.debian.net into
>>
>
>> +class UpdateDebianDuckTask(BaseTask): +""" +A task for
>> updating upstream url issue information on all packages. +
>> """ + +DUCK_LINK = 'http://duck.debian.net' +# URL of the
>> list of source packages with issues. +DUCK_SP_LIST_URL =
>> 'http://duck.debian.net/static/sourcepackages.txt' + +
>
> Single empty line here.
fixed.
>
>> +ACTION_ITEM_TYPE_NAME = 'debian-duck' +
>> ACTION_ITEM_TEMPLATE = 'debian/duck-action-item.html' +
>> ITEM_DESCRIPTION = 'The URL(s) for this package had some recent
>> persistent issues' + +def
>> __init__(self, force_update=False, *args, **kwargs): +
>> super(UpdateDebianDuckTask, self).__init__(*args, **kwargs) +
>> self.force_update = force_update + self.action_item_type =
>> ActionItemType.objects.create_or_update( +
>> type_name=self.ACTION_ITEM_TYPE_NAME, +
>> full_description_template = self.ACTION_ITEM_TEMPLATE)
>
> Please do not use "tabs" but real spaces with 4-space
done.
>
>
>> +def _get_duck_urls_content(self): + """ + Gets the list of
>> packages with URL issues from + duck.debian.net + + :returns: A
>> array if source package names. + """ + +ducklist =
>> get_resource_content(self.DUCK_SP_LIST_URL) + + packages = [] +
>> for package_name in ducklist.splitlines(): +
>> package_name = package_name.strip() +
>> packages.append(package_name) +return packages
>
> Eek, the mix of spaces and tabs gives a really bad output here
> when quoted...
also fixed.
>
> Also get_resource_content will return None when the content of the
> URL hasn't changed since last time, you should deal with that
> case.
>
>> + +
>
> A single empty line here.
>
>> +def update_action_item(self,package):
>
> Space after the comma.
fixed.
>
>> +action_item =
>> package.get_action_item_for_type(self.action_item_type) +if not
>> action_item: + action_item = ActionItem( + package =
>> package, + item_type = self.action_item_type, +) + +
>> issues_link = self.DUCK_LINK + "/static/sp/" + package.name[0] +
>> "/" + package.name + ".html"
>
> Are you really using a directory split based on the first character
> only? I ask because the debian pool has a special case for "lib*"
> packages where it uses the four first letters and many services
> have been mimicking this choice.
ok, fixed, i now use the lib* approach.
>
>> +action_item.short_description=
>> self.ITEM_DESCRIPTION.format(issues_link = issues_link)
>
> one space around the first "=" (in the assignation) but no spaces
> around the second one (named parameter).
>
>> +action_item.save() + +def execute(self): + ducklings=
>> self._get_duck_urls_content()
>
> One space around the equal.
fixed.
>
>> +if ducklings is None: + return
>
> Your current version of _get_duck_urls_content() will never return
> None but this is something to fix (cf another comment above).
>
fixed (at least to my understanding).
>> +ActionItem.objects.delete_obsolete_items( +
>> item_types=[self.action_item_type], +
>> non_obsolete_packages=ducklings) + + packages =
>> SourcePackageName.objects.filter(name__in=ducklings) + + for
>> package in packages: + self.update_action_item(package) +
>> class UpdateWnppStatsTask(BaseTask):
>
> Please put two empty lines be
Bug#758502: tracker.debian.org: Add data from duck.debian.net to "action needed" panel
-BEGIN PGP SIGNED MESSAGE- Hash: SHA512 Am 2014-08-19 11:26, schrieb Raphael Hertzog: > Control: forcemerge -1 756745 > > On Mon, 18 Aug 2014, Simon Kainz wrote: >> this adds support for data from duck.debian.net. > > There was already an open bug against tracker.d.o requesting the > support of duck.debian.net (and I think I pointed you to it on > IRC), so merging your bug with it. > > Cheers, > ok, thank you. My workstation rebooted due to power issues, so my IRC session vanished and i didn't see your message. Bye, -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.12 (GNU/Linux) iQIcBAEBCgAGBQJT9IKgAAoJEBy08PeN7K/pM5EQAIc0DYgZKx7VyxBC2yExEr1G RVawKrRByCipnvG3utqridB3awDsWefbgYOncqDUd3pHyxcNlslE25ep0NkMj/n6 EgE1VTMQY19j9IYabyjx1jszfaOTQQp5tqny5Z+jknQDT/aFXIEXJXJ0HffEeLSu dbsh6xeEsBgvelSZ+BVY2XPLGmqdQ2UtUTlHtfH6nZlaKI4/sTeon0VRJI8r4H9b K3q9AEXVI7KpVSVpvpd9cMBjcnZEyYsS3hNV7UpycrgXJL/u7Yu2NMoiG5fxSYQS YbTrgl8PcmeuxvVzPDjx3uvlBuNX4VycCvSNidco3jlpN6FWy824a4yTqMFvWcuH R6H8cVI0LpJ3av2LuVY4s+OfRBQH18ohrfrIiTGQG/lZAYxR8wY5iPNkxCwnjNqN i3gkzyiyroTbK4vT1qVSztmQfRiOncTmhtvOV5nMKS62zCbGuUvi/tOY/egJzfgg Go9z4ki1oa76CBoKeuY+QsVdJy5Ze7hl3rc6Vu3Bb1pWIJxZURK9Zz/BGwCtacHx zpnE0bH9TRsydWhsmcqSYy0BzuCB5ho/pXFM+R71yVZrrma+jXeL53LSIktQhbkk o1wvmykR5oVDNYTjMeOS6SXWkUST2UYPC7Bh5AsRoJD2XYhWMlHgWB9dlX/804qp 8KSlsItQvH2n/z9EUOJ0 =826n -END PGP SIGNATURE- -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org
Bug#758502: tracker.debian.org: Add data from duck.debian.net to "action needed" panel
Control: forcemerge -1 756745 On Mon, 18 Aug 2014, Simon Kainz wrote: > this adds support for data from duck.debian.net. There was already an open bug against tracker.d.o requesting the support of duck.debian.net (and I think I pointed you to it on IRC), so merging your bug with it. Cheers, -- Raphaël Hertzog ◈ Debian Developer Discover the Debian Administrator's Handbook: → http://debian-handbook.info/get/ -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org
Bug#758502: tracker.debian.org: Add data from duck.debian.net to "action needed" panel
Hello Simon,
On Mon, 18 Aug 2014, Simon Kainz wrote:
> this adds support for data from duck.debian.net.
Thanks for the patch! Here are a few things to fix:
First you should respect PEP8 for the coding style, notably:
- use only spaces to indent, not tabs
- spaces around assignation operatior
You can use "pep8" to check your code (distro-tracker is not fully PEP8
compliant yet, but we try to pay attention for new code).
> >From a0a3810608a306ab77eee214a9be0854e39e3367 Mon Sep 17 00:00:00 2001
> From: Simon Kainz
> Date: Mon, 18 Aug 2014 10:42:28 +0200
> Subject: [PATCH 1/2] add new template for duck extra info.
There's no point in adding the template in a separate commit. Please
squash both commits together.
> --- /dev/null
> +++ b/distro_tracker/vendor/debian/templates/debian/duck-action-item.html
> @@ -0,0 +1,8 @@
> +{% with duck = item.extra_data.duck_link %}
> +{% with issues = item.extra_data.issues_link %}
> +DUCK reports some issues
> concerning upstream URLs defined for this package.
> +{% endwith %}
> +{% endwith %}
The "with" tag doesn't bring anything as you use each variable only once.
Please inline the duck and issues variables.
> From 11e3918f8f9b99b96e0d77c11ddf686dc36d20f1 Mon Sep 17 00:00:00 2001
> From: Simon Kainz
> Date: Mon, 18 Aug 2014 10:43:14 +0200
> Subject: [PATCH 2/2] add data from duck.debian.net into
>
> +class UpdateDebianDuckTask(BaseTask):
> +"""
> +A task for updating upstream url issue information on all packages.
> +"""
> +
> +DUCK_LINK = 'http://duck.debian.net'
> +# URL of the list of source packages with issues.
> +DUCK_SP_LIST_URL = 'http://duck.debian.net/static/sourcepackages.txt'
> +
> +
Single empty line here.
> +ACTION_ITEM_TYPE_NAME = 'debian-duck'
> +ACTION_ITEM_TEMPLATE = 'debian/duck-action-item.html'
> +ITEM_DESCRIPTION = 'The URL(s) for this package had some recent
> persistent issues'
> +
> +def __init__(self, force_update=False, *args, **kwargs):
> + super(UpdateDebianDuckTask, self).__init__(*args, **kwargs)
> + self.force_update = force_update
> + self.action_item_type = ActionItemType.objects.create_or_update(
> + type_name=self.ACTION_ITEM_TYPE_NAME,
> + full_description_template = self.ACTION_ITEM_TEMPLATE)
Please do not use "tabs" but real spaces with 4-space
> +def _get_duck_urls_content(self):
> + """
> + Gets the list of packages with URL issues from
> + duck.debian.net
> +
> + :returns: A array if source package names.
> + """
> +
> +ducklist = get_resource_content(self.DUCK_SP_LIST_URL)
> +
> + packages = []
> +for package_name in ducklist.splitlines():
> +package_name = package_name.strip()
> +packages.append(package_name)
> +return packages
Eek, the mix of spaces and tabs gives a really bad output here when
quoted...
Also get_resource_content will return None when the content of the URL
hasn't changed since last time, you should deal with that case.
> +
> +
A single empty line here.
> +def update_action_item(self,package):
Space after the comma.
> + action_item = package.get_action_item_for_type(self.action_item_type)
> + if not action_item:
> + action_item = ActionItem(
> + package = package,
> + item_type = self.action_item_type,
> + )
> +
> + issues_link = self.DUCK_LINK + "/static/sp/" + package.name[0] + "/" +
> package.name + ".html"
Are you really using a directory split based on the first character only?
I ask because the debian pool has a special case for "lib*" packages where
it uses the four first letters and many services have been mimicking this
choice.
> + action_item.short_description= self.ITEM_DESCRIPTION.format(issues_link
> = issues_link)
one space around the first "=" (in the assignation) but no spaces
around the second one (named parameter).
> + action_item.save()
> +
> +def execute(self):
> + ducklings= self._get_duck_urls_content()
One space around the equal.
> + if ducklings is None:
> + return
Your current version of _get_duck_urls_content() will never return None
but this is something to fix (cf another comment above).
> + ActionItem.objects.delete_obsolete_items(
> + item_types=[self.action_item_type],
> + non_obsolete_packages=ducklings)
> +
> + packages = SourcePackageName.objects.filter(name__in=ducklings)
> +
> + for package in packages:
> + self.update_action_item(package)
> +
> class UpdateWnppStatsTask(BaseTask):
Please put two empty lines between two class definitions.
It would also be nicer if you could include unit tests for your code. It
should relatively easy to adapt from other similar tasks in
distro_tracker/vendor/debian/tests.py (UpdateDebciStatusTask for example).
Cheers,
--
Raphaël Hertzog ◈ Debian Developer
Discover the Debian Administrator's Handbook:
→ http://debian-handbo
Bug#758502: tracker.debian.org: Add data from duck.debian.net to "action needed" panel
Package: tracker.debian.org
Severity: wishlist
Tags: patch
Hello,
this adds support for data from duck.debian.net.
Please see the attached patch.
-- System Information:
Debian Release: 7.5
APT prefers stable
APT policy: (500, 'stable')
Architecture: amd64 (x86_64)
Kernel: Linux 3.2.0-4-amd64 (SMP w/8 CPU cores)
Locale: LANG=de_AT.UTF-8, LC_CTYPE=de_AT.UTF-8 (charmap=UTF-8)
Shell: /bin/sh linked to /bin/dash
>From a0a3810608a306ab77eee214a9be0854e39e3367 Mon Sep 17 00:00:00 2001
From: Simon Kainz
Date: Mon, 18 Aug 2014 10:42:28 +0200
Subject: [PATCH 1/2] add new template for duck extra info.
---
.../vendor/debian/templates/debian/duck-action-item.html | 8
1 file changed, 8 insertions(+)
create mode 100644 distro_tracker/vendor/debian/templates/debian/duck-action-item.html
diff --git a/distro_tracker/vendor/debian/templates/debian/duck-action-item.html b/distro_tracker/vendor/debian/templates/debian/duck-action-item.html
new file mode 100644
index 000..851310c
--- /dev/null
+++ b/distro_tracker/vendor/debian/templates/debian/duck-action-item.html
@@ -0,0 +1,8 @@
+{% with duck = item.extra_data.duck_link %}
+{% with issues = item.extra_data.issues_link %}
+DUCK reports some issues concerning upstream URLs defined for this package.
+{% endwith %}
+{% endwith %}
+
+
+
--
2.0.1
>From a0a3810608a306ab77eee214a9be0854e39e3367 Mon Sep 17 00:00:00 2001
From: Simon Kainz
Date: Mon, 18 Aug 2014 10:42:28 +0200
Subject: [PATCH 1/2] add new template for duck extra info.
---
.../vendor/debian/templates/debian/duck-action-item.html | 8
1 file changed, 8 insertions(+)
create mode 100644 distro_tracker/vendor/debian/templates/debian/duck-action-item.html
diff --git a/distro_tracker/vendor/debian/templates/debian/duck-action-item.html b/distro_tracker/vendor/debian/templates/debian/duck-action-item.html
new file mode 100644
index 000..851310c
--- /dev/null
+++ b/distro_tracker/vendor/debian/templates/debian/duck-action-item.html
@@ -0,0 +1,8 @@
+{% with duck = item.extra_data.duck_link %}
+{% with issues = item.extra_data.issues_link %}
+DUCK reports some issues concerning upstream URLs defined for this package.
+{% endwith %}
+{% endwith %}
+
+
+
--
2.0.1
duck_patches.tgz
Description: GNU Zip compressed data
