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

Re: RFS: golang-github-humanlogio-humanlog/0.7.6-1 [ITP] -- Logs for humans to read.



-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

Hi Nilesh,

On Mon, 2023-11-20 at 20:23 +0530, Nilesh Patra wrote:
> You could simply package https://github.com/humanlogio/api and there is
> no need to package a subdirectory. However, this is filled with pb.go
> files and I concur that it feels weird to package. This seems OK to
> vendor.

humanlog imports github.com/humanlog/api/go (the *go* subdirectory), and there's
no go.mod in the root directory of the repo. I don't know why, but dh-make-
golang complains that there aren't any Go files, both when run on
github.com/humanlog/api and when run on copy of the repo I made that has all the
stuff in the go/ directory moved out (mv go/* .).

> However, the major issue here is that there is no license on that repo
> and that is a ground for reject.

I'm assuming it's under the same license as humanlog itself (since it's a
component of humanlog), and that is Apache-2.0. Should I clarify upstream?

> Your copyright is also incomplete - do note that any other component
> that has an author and/or license needs to be clearly mentioned in the
> copyright. I had fixed the copyright for you in kr-logfmt for instance
> in case you checked. 

I did see, thanks!

> You could also look at the d/copyright of kitty
> itself.

I just did a quick grep -r for any mentions of the word 'copyright' and nothing
else came up (it only got the LICENSE file and stuff in vendor/), so I think
that d/copyright is complete. The source files for both humanlog and humanlogio-
api don't have copyright headers, so I'm assuming they're all under Apache-2.0.

> Additionally, the vendor/ directory contains tons of un-needed stuff.
> Kindly repack it to include only needed humanlgio/api package.
> 
> 	https://wiki.debian.org/Javascript/Repacking
> 
> You could look at singularity-container package as an example.

Thanks for the pointer! dh-make-golang did most of the work, I've just changed
d/copyright so it doesn't exclude humanlogio-api, and I've done the repack.
Please have a look!

> >  * The version included in vendor/ is the right version for humanlog to work   
> > with.
> 
> I am not really sure what this is supposed to mean.

i.e. humanlog bundles the version of humanlogio-api it needs, v0.0.0-
20230114081617-9054c2994bcc (as set out in go.mod), nothing older, nothing
newer.

> >  * IMHO this should be internalised (a part of humanlog's codebase instead of 
> >    being separate).
> 
> Feel free to tell that to upstream with an issue.

Done[1].

> > I've achieved this by symlinking the relevant directory from vendor/ to the
> > build directory (_build/). See the VCS for the implementation.
> 
> I think the go build system would do it for you and the entire
> override_dh_auto_configure section can go away. The package will build just fine
> without any of that; and it is only complicating things in what is
> supposed to be a simple package.

That simplifies things, I've removed that section from d/rules. 

> > Note that (at the time of writing), the dependencies golang-github-kr-logfmt-dev
> > and golang-connectrpc-connect-dev are only available in unstable, and haven't
> > yet migrated to testing. Keep that in mind when working on this package (e.g.
> > adding unstable to sbuild chroot's sources.list).
> 
> You're actually supposed to build in an unstable chroot only.

My mistake :) I thought it built on testing.

[1]: https://github.com/humanlogio/humanlog/issues/72

Kind regards,
Maytham


-----BEGIN PGP SIGNATURE-----

iQIzBAEBCgAdFiEESl/RzRFQh8wD3DXB1ZeJcgbF8H8FAmVcagcACgkQ1ZeJcgbF
8H/sfw//ZoUdWzrXhnuLoQSCmP8dO+JuSOQu7bdbRLZWOD2i5mntcpLns6xUPsEa
OL2F/B/ayeQpE0XrlLCytfEb8gG5o7WoM8OmgGmgoZYp4odpQX1BdgpMGirNLVFp
6e+a3PN0iv6ro751ZWaNgA895Q3h0atA/lD6udngsvFQ/bICFsjBG9Oz7/j+OXy9
p6S0dleFUYyzAziNioywY/KnrMpCUtIRxvJoEnoefZ9pCfvfaX8jnRAOXiR43EjE
P7prmxULIY/EbmvYRzLGDimBEUiKo1e6FO2zdhhAz7ZFpXhXrNSLtsaRR4mzRZGn
RxldrQSVbNyNfp+JcAbhPQuqBuRUdeUfZGMtVIEJq4BGR4+SzVQRI31KQSZjEdKT
c+RTsy0tEznPmCiwS8o/3HIecMMnrHpkT8t4ELofyrUcNYMOI8aqxmVR0MC2nQU/
VkvOqrscpqmwAOAtfJD91cqg2Qbu+HL1HnFc9xBByuSZHadWj2W+yrVjk+CVnk4X
ClnD2x/WtNIBRJ3xnCU0KO0PbmpBlKSfPMnUgkJ7y5SFzUEzpl6YDhj1TRiV1s0n
FqnRcvNUFf0xCBgVaY8+3taOAZvNBAOBYx/R9/xBVNdsYVtB+8GK3RkGZBuxKvfg
LMXGQH0Ln2Jq7lZ3InFdhCaJsyaeO2mrcBdGCHMv0T5HjgwiyKc=
=2bSw
-----END PGP SIGNATURE-----


Reply to: