https://bugzilla.redhat.com/show_bug.cgi?id=2428704
--- Comment #20 from Rodolfo Olivieri <[email protected]> --- Spec URL: https://r0x0d.fedorapeople.org/goose/goose.spec SRPM URL: https://r0x0d.fedorapeople.org/goose/goose-1.23.2-4.fc45.src.rpm Generate tarball script: https://r0x0d.fedorapeople.org/goose/generate-vendor-tarball.sh Before getting to the above comments, I wanted to include the following changes from myself: * Re-wrote some of the comments in the hope to make them more clear and descriptive * Added a couple of missing links (like legal ML threads) * The release bumps in the srpm is because I'm tracking the work in a git repository (and we have made a few commits to it already, like adding packit in our upstream git for build+tests and some other experiments) ---- Now, to the good part: From Neal comments above; > %description > an open source, extensible AI agent that goes beyond code suggestions - > install, execute, edit, and test with any LLM. > This should be formulated in the form of proper English sentences. Updated the description of the package. > %autosetup -n %{name}-%{version} -p1 -a1 > This can be simplified to "%autosetup -p1 -a1". Removed the `-n %{name}-%{version}` from the %autosetup ------ From Fabio comments above; > You define a "stable_version" macro and use it exactly once (to set Version). > You could just omit that useless indirection and keep the comment above it. Removed the "stable_version" macro and moved the comments to be above "Version:". > a. You reference a "generate-vendor-tarball.sh" script that is used to > generate the tarball with vendored sources. > But this script is not included - you should include this as a "Source" file > so it is included in `.src.rpm` files. Added it as "Source2" and provided a comment on the reasoning for it. Let me know if the comment would be good to be added to the script itself. > b. The license files for included JavaScript libraries are now included > (good!) > but the Cargo.toml change is wrong (and unnecessary): > https://github.com/block/goose/commit/37f419d Okay, gotcha! I will open a follow-up PR for upstream and remove the Cargo.toml changes. > Also I wonder why you thought "cargo package --list" would be a good > "testing" indicator > for a subproject that is not published on crates.io. Googling around I found out that this command would print the list of files included in a build, but it makes sense that since this is not being published to crates.io, it would not matter. I just thought of including it in the case of they are eventually moving to crates.io (for some reason). > c. The License tag is ... better, but still wrong / incomplete / contains > items that are unexplained: Alright, I addressed the ones you mentioned in this topic. I tried to write a script to help me re-order the license (I know, should not be needed, but I was experimenting and seeing if it would work), and unfortunately, I didn't paid close attention to the output, clearly I have messed up... I have done it manually this time and got rid of the script. > d. The given instructions for generating the list of bundled Sublime Text > grammars and themes are broken. Cleaned that up as well. Instead of cat + xxd, now I wrote to use strings + grep. Looks much better and output a correct list of syntaxes. I have also removed the incorrect mention of "Provides: bundled(sublime-syntax-Text)" from it. -- 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=2428704 Report this comment as SPAM: https://bugzilla.redhat.com/enter_bug.cgi?product=Bugzilla&format=report-spam&short_desc=Report%20of%20Bug%202428704%23c20 -- _______________________________________________ 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
