https://bugzilla.redhat.com/show_bug.cgi?id=2431992
--- Comment #6 from Attila Lakatos <[email protected]> --- Hello, thank you very much for your inputs. (In reply to Fabio Valentini from comment #5) > I am at CentOS Connect / FOSDEM, I will probably only have time to look at > this more next week. > > Just a few quick comments: > > 1. This should not be necessary (or actually be superfluous / unused) when > not using vendored dependencies: > > > BuildRequires: openssl-devel > > # Use system OpenSSL instead of building from source > > export OPENSSL_NO_VENDOR=1 You are right, I removed the build requirements. > > 2. The patches are not documented (or at least not in the spec file, which > they should be). > Also, if any of them are upstreamable, that should happen and links to the > upstream PRs or commits should be added. I added brief comments for each patch in the spec file. Each patch itself contains a detailed description. This is one of the reasons I use git-style patches, as they allow additional information to be included. I agree on upstreaming some of the patches. Some already are but others won't be needed in the future. The whole trustee project has several components. The most important one for us is the KBS (key broker service). In future versions we want to support the other components as well such as the RVPS & attestation service. I wanted to give you a small insights into that, so you know why we are not including some of the components and why we have multiple downstream patches. > > 3. The License tag looks very big and contains duplicates - it doesn't > exactly look *wrong* but it's also messy. > > For example, you don't need to have both (Apache-2.0 OR MIT) *and* (MIT OR > Apache-2.0), they're equivalent. You could drop one of them (I usually keep > the one that sorts first alphabetically). > > Also, Things like "((Apache-2.0 OR MIT) AND BSD-3-Clause) AND ..." can be > flattened (AND is associative and the order doesn't matter), so those are > two more duplicates you can avoid. > > You can also use syntax like this to avoid the very long line (and better > diff it): > > License: %{shrink: > Apache-2.0 > AND MIT > AND BSD-3-Clause > AND ... > } Great note, thank you. Changed that. -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component https://bugzilla.redhat.com/show_bug.cgi?id=2431992 Report this comment as SPAM: https://bugzilla.redhat.com/enter_bug.cgi?product=Bugzilla&format=report-spam&short_desc=Report%20of%20Bug%202431992%23c6 -- _______________________________________________ package-review mailing list -- [email protected] To unsubscribe send an email to [email protected] Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedoraproject.org/archives/list/[email protected] Do not reply to spam, report it: https://forge.fedoraproject.org/infra/tickets/issues/new
