[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#806740: tracker.debian.org: Please add AppStream hints panel



On Sun, 22 May 2016, Matthias Klumpp wrote:
> > We don't want this model. It's basically a specialized version
> > of PackageExtractedInfo. Please use PackageExtractedInfo to store
> > your data related to each package (with a key="appstream" or similar).
> 
> Okay - I modeled this after the Lintian bits, so maybe Lintian should
> use that model too?

Yes, certainly. There are large parts of the current code that could
benefit from improvements. They were written during the initial summer
of code and we did not have the same infrastructure yet.

> > Django has a pluralize filter for this kind of problems:
> > https://docs.djangoproject.com/en/1.9/ref/templates/builtins/#pluralize
> 
> Neat! (Maybe something the Lintian code should use too then ^^)

Certainly. You're welcome to propose fixes for those too :-)

> > About the tests, they might be a bit too extensive or they need some
> > refactoring to avoid duplication, I haven't checked in details.
> 
> It's (almost) the same tests used for the Lintian code, which I though
> were very extensive too, but on the other hand nobody has ever
> complained about too many tests - until now ^^

Yeah, I won't block anything on this but really tests should be short and
readable and I have the feeling that we can do better here.

> > Also you might want to use static data stored in "tests-data" instead
> > of duplicating the same data in each test. We have a helper method
> > to locate data files from those directories (get_test_data_path).
> 
> That would make sense for longer data which really is the same - most
> of the JSON snippets used in the tests have some variations.
> I'll go through that again.

Yeah, but I find it clearer to load the common structure and manually
modify the part of the JSON that you want to alter specifically for the
test. That way the modification stands out rather clearly. Thus you can
even factor the initial JSON data in the setUp() method.

Cheers,
-- 
Raphaël Hertzog ◈ Debian Developer

Support Debian LTS: http://www.freexian.com/services/debian-lts.html
Learn to master Debian: http://debian-handbook.info/get/


Reply to: