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

Re: Working with gbp and older releases



Am Mittwoch, den 19.02.2014, 20:00 +0100 schrieb Dariusz Dwornikowski
> I uploaded my version to mentors. Would you be so nice to review it ? 
> http://mentors.debian.net/package/maradns

Hi Dariusz,

Sure, I'll take a look.  (I'm not a DD, so I cannot sponsor.)  

A suggestion: You should the RFS Bug to get a broader audience, maybe
someone who can indeed sponsor.

Ok, lets jump into the package: (Note that I do not sort the items, I
just write as I see them; so no ordering, severity, ... implied. And
don't get shocked by the length of the remarks, thats normal for the
first shot.)

-> Please file a bug for your changelog entry "Several security bugs" to
document in the BTS that the current version in xxx has problems. 
(One bug is enough, just quote your 5-lines in the changelog as bug
report)

-> For the old changelogs, where you added for example

  [ Dariusz Dwornikowski ]
  * Security bugfix for CVE-2012-1570

This is somehow misleading as it implies that you actively did smth on
the pckageing, but leave it unclear "what has been done". I think you
should not add your name here and hadd the "CVE-" to the (existing)
changelog entry. In this special case I would just update the first line
to

maradns (2.0.06-1) experimental; urgency=low

  * New upstream release, fixes CVE-2012-1570

(And then document the change in d/changelog for the to-be-uploaded
version, for example
 
  * Updated old d/changelog entries: Added information when the CVEs   
    where fixed: (adding Debian-Versions which where changed=  

If you bump the SV, you should not if there have been any changes
necessary and if not document that too.

Generally, d/changelog entries should reflect *what* have been changed
on not focus on the *why* something has changed. For example, you write
"We no longer modify the config (Closes: #710903)": 
I would write "updated d/postinst to no longer modifiy conffiles.
(Closes: #...)
(and in the patch for it, do not just comment out the lines, remove them
to be clear that this is not acciedentially commented out)

-> Regarding the usage of your repository. I would suggest to have "one
change - one commit" and also have the commit messages speak for the
changes (ideally, they are identical to the d/changelog entry); There
were at least some commit which changed more than the git log says.
(the commit for the conffile fix is a examples - it also changes
d/control but does not mention it)
(BTW, in this commit you change the dependency of duende to an *older*
version? As this looks weird, I *suppose* this is wrong... Maybe you
wanted to enforce the current version, then use ${binary:Version})

I see also that there are sometimes undocumented changes, please
document them. For example you updated the watchfile, add gpg signatures
but this is not documented in the d/changelog. But this is just an
example, there are more than this one. 

d/control:
Why is the Breaks / Replaces necessary for maradns-zoneserver and other
packages? Why does the docs breaks an older version of maradns?

In Debian there is a file DwRandPrime.h which seems somehow
autogenerated. What is its purpose?


((note, I had to stop the review here due to time constraints)


Best regards, 
Tobias


Reply to: