On Tue, 24 Mar 2020, 19:56 Andreas Henriksson, <[email protected]> wrote:
> Hi, > > Thanks for your feedback. Minor comment below. > > On Tue, Mar 24, 2020 at 07:13:14PM +0530, Nilesh Patra wrote: > > 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. > > No worries. Thanks for telling me the status about this. > :-) > I've not personally used it but it sounds for your situation that > deb-o-matic might be something you're interested in using. > Ah, didn't know about this. Sounds good! See also currently ongoing discussion about 'ratt as a service' > on debian-devel. Personally I think it would be nice to have > possibility to trigger ratt manually when needed on a salsa/gitlab > CI pipeline. Unfortunately this doesn't exist (yet), so for now > people mostly use their own resources for rebuilds. > Yep, I read the discussion on the list, it would personally make it a lot easier for me too. Hope it gets implemented soon. > > > - 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? > > If upstream adds one it'll be easy to mistakenly import them in future > updates. I'd say it's safer to keep the Files-Excluded for consistency > within the team packages and as a safety precaution for future imports. > Better safe than sorry. > Yep, and it seems you have already reverted the commit, thanks :) Please consider uploading this, as and when it's convenient for you. Regards, Nilesh
