https://bugzilla.redhat.com/show_bug.cgi?id=2463266



--- Comment #9 from Fabio Valentini <[email protected]> ---
Sorry for the delay. I reviewed unidiff and wanted to wait with this one until
it was available in the rawhide repos, and then took a few days off work and
forgot.

The package looks pretty good now, just a few minor comments:

1. I would recommend to copy the contents between "### BEGIN LICENSE SUMMARY
###" and "###  END LICENSE SUMMARY  ###" into the spec file above the License
tag. It helps keeping track and with seeing an obvious diff when updating the
License tag on package update.

Something like this:

```SourceLicense:  MIT
# (Apache-2.0 OR MIT) AND BSD-3-Clause
# (MIT OR Apache-2.0) AND Unicode-DFS-2016
# Apache-2.0 OR MIT
# MIT
# MIT AND Unicode-DFS-2016 AND BSD-2-Clause AND BSD-3-Clause AND
LicenseRef-Fedora-Public-Domain
# MIT OR Apache-2.0
# MPL-2.0
# Unlicense OR MIT
License:        %{shrink:
        (MIT OR Apache-2.0) AND Unicode-DFS-2016 AND
        (Apache-2.0 OR MIT) AND
        (MIT AND Unicode-DFS-2016 AND BSD-2-Clause AND BSD-3-Clause AND
LicenseRef-Fedora-Public-Domain) AND
        (MIT OR Apache-2.0) AND
        MIT AND
        MPL-2.0 AND
        (Unlicense OR MIT)
}
# LICENSE.dependencies contains a full license breakdown
```

On the topic of License tag, it looks OK but it also contains duplicates and
could be slightly simplified (like dropping duplicate "AND" clauses:

```
License:        %{shrink:
    MIT
    AND BSD-2-Clause
    AND BSD-3-Clause
    AND MPL-2.0
    AND Unicode-DFS-2016
    AND LicenseRef-Fedora-Public-Domain
    AND (Apache-2.0 OR MIT)
    AND (Unlicense OR MIT)
}
```

This applies (almost) all simplifications that are documented to be valid:
 1. flatten nested AND clauses (associativity)
 2. drop duplicates when both "A OR B" and "B OR A" are present since they're
considered equivalent (commutativity)


2. The "tree-sitter" crate was now updated to version 0.26 in Fedora 44+, you
*could* drop the downstream patch that downgrades the dependency. But you can
also keep it for now, that would make the package also compatible with Fedora
43 if that's what you want to do.


-- 
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=2463266

Report this comment as SPAM: 
https://bugzilla.redhat.com/enter_bug.cgi?product=Bugzilla&format=report-spam&short_desc=Report%20of%20Bug%202463266%23c9

-- 
_______________________________________________
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

Reply via email to