Hi,
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