Re: RFS for 6 go packages
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, <andreas@fatal.se> 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.
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.
>
> - 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.
>
> 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
>
> >
Regards,
Andreas Henriksson
Reply to: