Hi, On Tue, 24 Mar 2020, 18:36 Andreas Henriksson, <[email protected]> wrote:
> Hello again, > > On Sun, Mar 22, 2020 at 09:12:09PM +0530, Nilesh Patra wrote: > [...] > > * golang-go.uber-zap [3] - This also has RFA, and hence I have added my > > name to uploaders. > [...] > > [3]: https://salsa.debian.org/go-team/packages/golang-go.uber-zap > [...] > > The upstream/1.14.1 tag is missing in the git repo, please make sure to > push tags. Pushed for all the updates. (gbp push will help you DTRT. Also as you mentioned I agree > it's better if the uploader pushes the debian/* tag when the package > is actually uploaded, but other tags like upstream/* you really have > to push. OTOH it's also ok by me if you push the debian/* tag. When > you're the maintainer you decide about releases, even if some of them > mighht never make it into the official archive.) > Thanks, but I'll leave that to the uploader to do it, as a good practice. > Some things to note that would make it easier for a drive-by sponsor > like me to review your work: > > * New upstream release. > > I don't trust golang stuff to not break API every release, so please > tell me (in your RFS) if you've: > - investigated possible API breaks > I did, but didn't find any 'breaking' changes here. - tested building reverse dependencies (see ratt) > No, I didn't do this. I admit that it's a little difficult for me to do, with the limited bandwidth that I have as of now. I'll try doing this whenever I can, then. Btw, is this a (major) package which can break a lot of stuff? If it isn't, then I'll prefer to skip ratt, as it's beyond my current computation power and bandwidth to do; also I'm not sure if this would bring in major changes, since this package was already broken. Apologies if this doesn't sound good to you. - any other useful info related to the new release that I should be > aware of (like any particular reason for updating it). > I updated this to keep it close to upstream version, no other ulterior motive, really. > * debian/compat to debhelper-compat > > - You forgot to mention that you also bumped compat from 10 to 12! > This is more important to mention than changing from file to > virtual package dep. (See also lintian-brush which will help > you do these kinds of changes and DTRT with commit messages.) > Noted. > * debian/copyright changes > > Please mention all changes in debian/changelog !!! > The commit message doesn't help me much either. Please write good > commit messages describing not only what you did but also why. > Since this wasn't changing a lot, I didn't write the commit message explaining the 'why' of it, but thanks for pointing out, and I'll keep this in mind from now on. Please use 'gbp dch' to generate debian/changelog which will > help you make sure to mention all changes in the changelog > as well as making sure your commit messages has the quality > that is expected from debian/changelog messages. > I still don't know if I agree with dropping the Files-Excluded > change (and also don't know your reason for doing that). > I looked up upstream and didn't find the vendor directory or Godeps there, and hence removed them. Should this change not have been done? I guess these are all minor nitpicks and I can probably find more even > smaller ones, but as mentioned... everyone is busy and sponsorship > is a limited resource, so please make your stuff easy to review. Making > it easy for sponsors means you get more of your stuff sponsored. > Thanks a lot! That was an exhaustive review! :) You really pointed out minor stuff that I need to change, and I'll keep that in mind for sure. That said, if you would agree with me here, can you sponsor this one too? Thanks a lot, Nilesh >
