[Bug 1923830] Review Request: diffuse - Diff Utility
https://bugzilla.redhat.com/show_bug.cgi?id=1923830 --- Comment #24 from niohiani --- All good now. New version of diffuse is in now in Rawhide, and should be in f33 and f34 quite soon: https://src.fedoraproject.org/rpms/diffuse Thanks for all the help and guidance! I look forward to getting more involved in packaging and package maintenance as I continue to learn more. -- 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 ___ package-review mailing list -- package-review@lists.fedoraproject.org To unsubscribe send an email to package-review-le...@lists.fedoraproject.org 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/package-review@lists.fedoraproject.org Do not reply to spam on the list, report it: https://pagure.io/fedora-infrastructure
[Bug 1923830] Review Request: diffuse - Diff Utility
https://bugzilla.redhat.com/show_bug.cgi?id=1923830 Ankur Sinha (FranciscoD) changed: What|Removed |Added Summary|Review Request: Diffuse - |Review Request: diffuse - |Diff Utility|Diff Utility |(Re-introducing Retired | |Package)| -- 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 ___ package-review mailing list -- package-review@lists.fedoraproject.org To unsubscribe send an email to package-review-le...@lists.fedoraproject.org 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/package-review@lists.fedoraproject.org Do not reply to spam on the list, report it: https://pagure.io/fedora-infrastructure
[Bug 1923830] Review Request: Diffuse - Diff Utility (Re-introducing Retired Package)
https://bugzilla.redhat.com/show_bug.cgi?id=1923830 --- Comment #23 from Gwyn Ciesla --- (fedscm-admin): The Pagure repository was created at https://src.fedoraproject.org/rpms/Diffuse -- 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 ___ package-review mailing list -- package-review@lists.fedoraproject.org To unsubscribe send an email to package-review-le...@lists.fedoraproject.org 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/package-review@lists.fedoraproject.org Do not reply to spam on the list, report it: https://pagure.io/fedora-infrastructure
[Bug 1923830] Review Request: Diffuse - Diff Utility (Re-introducing Retired Package)
https://bugzilla.redhat.com/show_bug.cgi?id=1923830 --- Comment #22 from niohiani --- Many thanks! I'll jump on this ASAP. -- 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 ___ package-review mailing list -- package-review@lists.fedoraproject.org To unsubscribe send an email to package-review-le...@lists.fedoraproject.org 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/package-review@lists.fedoraproject.org Do not reply to spam on the list, report it: https://pagure.io/fedora-infrastructure
[Bug 1923830] Review Request: Diffuse - Diff Utility (Re-introducing Retired Package)
https://bugzilla.redhat.com/show_bug.cgi?id=1923830 --- Comment #21 from Ankur Sinha (FranciscoD) --- Hello, I've now sponsored you to the package maintainers group. Please continue from here: https://fedoraproject.org/wiki/Join_the_package_collection_maintainers#Add_Package_to_Source_Code_Management_.28SCM.29_system_and_Set_Owner Cheers, Ankur -- 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 ___ package-review mailing list -- package-review@lists.fedoraproject.org To unsubscribe send an email to package-review-le...@lists.fedoraproject.org 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/package-review@lists.fedoraproject.org Do not reply to spam on the list, report it: https://pagure.io/fedora-infrastructure
[Bug 1923830] Review Request: Diffuse - Diff Utility (Re-introducing Retired Package)
https://bugzilla.redhat.com/show_bug.cgi?id=1923830 --- Comment #20 from niohiani --- P.P.S. I had to travel for a bit longer than expected due to a family situation, but am back home now and will get on top of everything over the next few days. Sincere apologies for the delay. -- 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 ___ package-review mailing list -- package-review@lists.fedoraproject.org To unsubscribe send an email to package-review-le...@lists.fedoraproject.org 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/package-review@lists.fedoraproject.org Do not reply to spam on the list, report it: https://pagure.io/fedora-infrastructure
[Bug 1923830] Review Request: Diffuse - Diff Utility (Re-introducing Retired Package)
https://bugzilla.redhat.com/show_bug.cgi?id=1923830 --- Comment #19 from niohiani --- Final revision of spec file is here: https://download.copr.fedorainfracloud.org/results/niohiani/Diffuse-Packaging-Test/fedora-rawhide-x86_64/02210632-diffuse/diffuse.spec SRPM is here: https://download.copr.fedorainfracloud.org/results/niohiani/Diffuse-Packaging-Test/fedora-rawhide-x86_64/02210632-diffuse/diffuse-0.6.0-1.fc35.src.rpm Pull request in the Diffuse repository for correcting the FSF mailing address is here: https://github.com/MightyCreak/diffuse/pull/97/commits/4abb8be85b5bd2f769db632b93a2f5252424ca89 I will continue on with importing Diffuse according to the link you posted, but want to make sure I have the time to do it all correctly, so it might take a few days (I'm traveling tomorrow morning). I will also start pitching in with package reviews as well, but that might take a few days too, as I'll be with my travel laptop, and feel like I should pick simpler packages so I can actually be useful. Thanks so much again Ankur, and I shall be in touch very soon. P.S. The package testing COPR repo will be removed once I complete the aforementioned tasks. -- 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 ___ package-review mailing list -- package-review@lists.fedoraproject.org To unsubscribe send an email to package-review-le...@lists.fedoraproject.org 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/package-review@lists.fedoraproject.org Do not reply to spam on the list, report it: https://pagure.io/fedora-infrastructure
[Bug 1923830] Review Request: Diffuse - Diff Utility (Re-introducing Retired Package)
https://bugzilla.redhat.com/show_bug.cgi?id=1923830 --- Comment #18 from niohiani --- Many thanks Ankur! I'll make a pull request in MightyCreak's repo to correct the address in the license file, and of-course reduce the relevant entries under %files to that one line. I'll get into pitching in with those package reviews ASAP as well. -- 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 ___ package-review mailing list -- package-review@lists.fedoraproject.org To unsubscribe send an email to package-review-le...@lists.fedoraproject.org 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/package-review@lists.fedoraproject.org Do not reply to spam on the list, report it: https://pagure.io/fedora-infrastructure
[Bug 1923830] Review Request: Diffuse - Diff Utility (Re-introducing Retired Package)
https://bugzilla.redhat.com/show_bug.cgi?id=1923830
Ankur Sinha (FranciscoD) changed:
What|Removed |Added
Status|ASSIGNED|POST
Flags|fedora-review? |fedora-review+
--- Comment #17 from Ankur Sinha (FranciscoD) ---
Sorry for the delay---got very busy all of a sudden.
OK, that looks good! There are one or two items remaining, but you can do those
before you import the package.
- in the files section, you only need the one line (since it implies that it
also owns all sub-directories):
%{_datadir}/gnome/help/%{name}
instead of
%{_datadir}/gnome/help/%{name}
%{_datadir}/gnome/help/%{name}/C
%{_datadir}/gnome/help/%{name}/cs
%{_datadir}/gnome/help/%{name}/it
%{_datadir}/gnome/help/%{name}/ru
- rpmlint shows that the FSF address in the license file is wrong (it uses the
old address and needs to be updated)
diffuse.noarch: E: incorrect-fsf-address /usr/share/licenses/diffuse/COPYING
please file a ticket upstream to ask them to update this (please do not update
the address yourself, since we must use the license file in the form that
upstream provides it).
XXX APPROVED XXX
With regards to sponsorship to the packagers group, could you review one or two
tickets please? Reviewing packages for colleagues is as much a duty of us
package maintainers as maintaining our own packages is. So you also need to
know how to do that. You can use `fedora-review` to help you.
I guess you continue from this bit:
https://fedoraproject.org/wiki/Join_the_package_collection_maintainers#Add_Package_to_Source_Code_Management_.28SCM.29_system_and_Set_Owner
I'll be happy to help with your reviews too, of course. Please feel free to CC
me to any tickets and ask me any questions on them (or directly over e-mail at
ankursinha AT fedoraproject.org)
.
You can find all tickets awaiting review here:
https://fedoraproject.org/PackageReviewStatus/
Please note in your reviews that you are awaiting sponsorship (since you cannot
approve packages until you are sponsored).
Cheers,
Ankur
--
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
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org
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/package-review@lists.fedoraproject.org
Do not reply to spam on the list, report it:
https://pagure.io/fedora-infrastructure
[Bug 1923830] Review Request: Diffuse - Diff Utility (Re-introducing Retired Package)
https://bugzilla.redhat.com/show_bug.cgi?id=1923830 --- Comment #16 from niohiani --- P.S. I noticed - because I always have the following program installed under any distro I use - that GChemTable also creates a directory within /usr/share/gnome/help under Fedora, so I did not list the entire /usr/share/gnome/help directory under %files in my spec. I did list all the other ones you mentioned though. -- 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 ___ package-review mailing list -- package-review@lists.fedoraproject.org To unsubscribe send an email to package-review-le...@lists.fedoraproject.org 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/package-review@lists.fedoraproject.org Do not reply to spam on the list, report it: https://pagure.io/fedora-infrastructure
[Bug 1923830] Review Request: Diffuse - Diff Utility (Re-introducing Retired Package)
https://bugzilla.redhat.com/show_bug.cgi?id=1923830 --- Comment #15 from niohiani --- OK, I jumped on this as quick as I could, and addressed the aforementioned issues, along with switching to the eMail address associated with my user account in the changelog. Spec: https://download.copr.fedorainfracloud.org/results/niohiani/Diffuse-Packaging-Test/fedora-rawhide-x86_64/02161033-diffuse/diffuse.spec SRPM: https://download.copr.fedorainfracloud.org/results/niohiani/Diffuse-Packaging-Test/fedora-rawhide-x86_64/02161033-diffuse/diffuse-0.6.0-1.fc35.src.rpm I'm not 100% sure about the validity of the method I used to specify that diffuse owns the directories you mentioned (placing them under the %files directive), so if there's something I should be doing differently in that regard, just let me know what it is and I'll make the necessary modifications. Also, I based the transition of the AppData files into the metainfo directory on commits I saw in other projects that needed to do the same (I learn best through direct examples), so if there is a better way to do it, I'm glad to make those modifications with the necessary instructions as well. -- 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 ___ package-review mailing list -- package-review@lists.fedoraproject.org To unsubscribe send an email to package-review-le...@lists.fedoraproject.org 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/package-review@lists.fedoraproject.org Do not reply to spam on the list, report it: https://pagure.io/fedora-infrastructure
[Bug 1923830] Review Request: Diffuse - Diff Utility (Re-introducing Retired Package)
https://bugzilla.redhat.com/show_bug.cgi?id=1923830 --- Comment #14 from niohiani --- Shoot. I thought I caught everything. Sorry for missing the debug_package line. I'll fix that. It's definitely noarch. I must have misunderstood what I read about %ghost, so I'll remedy that too. I admit that I got a bit confused about specifying directory ownership in the spec file, but after a little more reading I think I've got it (such directories should be present under %files), but please correct me if I'm wrong. I'll do some more reading about the AppData topic, and I'll remove the COPYING file from the %doc list. I'll amend the spec file and do another build on COPR in the next 24 hours, so forget about reviewing the spec and SRPM in the previous comment, as a new one is incoming. Thanks! -- 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 ___ package-review mailing list -- package-review@lists.fedoraproject.org To unsubscribe send an email to package-review-le...@lists.fedoraproject.org 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/package-review@lists.fedoraproject.org Do not reply to spam on the list, report it: https://pagure.io/fedora-infrastructure
[Bug 1923830] Review Request: Diffuse - Diff Utility (Re-introducing Retired Package)
https://bugzilla.redhat.com/show_bug.cgi?id=1923830
--- Comment #13 from Ankur Sinha (FranciscoD) ---
Heya,
Looks pretty good. Will give it a full review again this week. Some tweaks that
I noticed that got left behind:
- please remove the "%global debug_package %{nil}" line. If the package is
noarch, it will not generate debuginfo. If it isn't noarch, it *must* generate
and include debuginfo.
- I don't think this is the right usage for the %ghost macro. As the docs say,
files that are generated by the package at runtime should be marked as %ghost.
The files here are not so---they are included in the rpm. Since no other
package in the Fedora repositories owns these directories, this package should
just own these directories, so please remove the %ghost directive there.
(Docs: http://ftp.rpm.org/max-rpm/s1-rpm-inside-files-list-directives.html)
- the appdata files now go to the metainfodir etc. Please see this:
https://docs.fedoraproject.org/en-US/packaging-guidelines/AppData/
- Also, just to double check: is the e-mail you are using in the change log
correct, since it does not match your e-mail here on bugzilla? (You should use
the e-mail you use here in Bugzilla, which should match the e-mail you provide
in the Fedora accounts system).
- since COPYING is marked by the %license tag, you do not need to repeat it in
the %doc list again.
Cheers,
Ankur
--
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
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org
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/package-review@lists.fedoraproject.org
Do not reply to spam on the list, report it:
https://pagure.io/fedora-infrastructure
[Bug 1923830] Review Request: Diffuse - Diff Utility (Re-introducing Retired Package)
https://bugzilla.redhat.com/show_bug.cgi?id=1923830
--- Comment #12 from niohiani ---
Alright Ankur! I think I've taken care of all the "Musts" by now, along with
the other things you pointed out. I did my best to address directory/file
ownership via the %ghost directive (which I just learned about); Increased the
use of macros as opposed to explicit paths; Marked the license file;
Consistently used %{buildroot} instead of $RPM_BUILD_ROOT; Edited the changelog
entries for readability. The dedicated repo has also allowed me to remedy the
versioning issue.
New testing repo for packaging:
https://copr.fedorainfracloud.org/coprs/niohiani/Diffuse-Packaging-Test/
Spec:
https://download.copr.fedorainfracloud.org/results/niohiani/Diffuse-Packaging-Test/fedora-rawhide-x86_64/02159737-diffuse/diffuse.spec
SRPM:
https://download.copr.fedorainfracloud.org/results/niohiani/Diffuse-Packaging-Test/fedora-rawhide-x86_64/02159737-diffuse/diffuse-0.6.0-1.fc35.src.rpm
If I've missed anything, I'll of course be happy to continue amending the .spec
file. Thanks so much for your patience.
--
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
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org
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/package-review@lists.fedoraproject.org
Do not reply to spam on the list, report it:
https://pagure.io/fedora-infrastructure
[Bug 1923830] Review Request: Diffuse - Diff Utility (Re-introducing Retired Package)
https://bugzilla.redhat.com/show_bug.cgi?id=1923830 --- Comment #11 from niohiani --- Thanks again, again, Ankur! I'll get on this very soon, and after this round of corrections it should all be in order. I'll create a temporary dedicated COPR repo just for getting the packaging up to snuff, so there's no conflicts for myself and the handful of others who use my repos. -- 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 ___ package-review mailing list -- package-review@lists.fedoraproject.org To unsubscribe send an email to package-review-le...@lists.fedoraproject.org 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/package-review@lists.fedoraproject.org Do not reply to spam on the list, report it: https://pagure.io/fedora-infrastructure
[Bug 1923830] Review Request: Diffuse - Diff Utility (Re-introducing Retired Package)
https://bugzilla.redhat.com/show_bug.cgi?id=1923830
--- Comment #10 from Ankur Sinha (FranciscoD) ---
Almost there, a little more work needed :)
Package Review
==
Legend:
[x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated
[ ] = Manual review needed
Issues:
===
- Package uses either %{buildroot} or $RPM_BUILD_ROOT
Note: Using both %{buildroot} and $RPM_BUILD_ROOT
See: https://docs.fedoraproject.org/en-US/packaging-guidelines/#_macros
^
Please use one form, for consistency.
- If (and only if) the source package includes the text of the license(s)
in its own file, then that file, containing the text of the license(s)
for the package is included in %license.
Note: License file COPYING is not marked as %license
See: https://docs.fedoraproject.org/en-US/packaging-
guidelines/LicensingGuidelines/#_license_text
^
Please mark the license file using %license.
- Package does not use a name that already exists.
Note: A package with this name already exists. Please check
https://src.fedoraproject.org/rpms/diffuse
See: https://docs.fedoraproject.org/en-US/packaging-
guidelines/Naming/#_conflicting_package_names
^
This is OK, since this is a re-review
- Directory ownership issues, please see below.
- Please prefer this form for the Source etc:
https://docs.fedoraproject.org/en-US/packaging-guidelines/SourceURL/#_git_tags
- please do not mix tabs and space: best to only use one form consistently
- if it's a noarch package, you do not need to disable debuginfo. Please remove
the line
- the appdata files now go to the metainfodir etc. Please see this:
https://docs.fedoraproject.org/en-US/packaging-guidelines/AppData/
- please correct the older changelog entries
- your spec/srpm don't match: the srpm version is 0.6.0-61? It should be
0.6.0-1?
= MUST items =
Generic:
[x]: Package is licensed with an open-source compatible license and meets
other legal requirements as defined in the legal section of Packaging
Guidelines.
[x]: License field in the package spec file matches the actual license.
Note: Checking patched sources after %prep for licenses. Licenses
found: "Unknown or generated", "GNU General Public License, Version
2", "*No copyright* GNU General Public License, Version 2". 101 files
have unknown license. Detailed output of licensecheck in
/home/asinha/dump/fedora-reviews/1923830-diffuse/licensecheck.txt
[!]: Package requires other packages for directories it uses.
Note: No known owner of /usr/share/gnome/help/diffuse/it,
/usr/share/gnome/help/diffuse/ru, /usr/share/omf/diffuse,
/usr/share/gnome/help/diffuse, /usr/share/gnome/help/diffuse/C,
/usr/share/gnome/help/diffuse/cs
^ Please check these directories for ownership. Nothing seems to own
/usr/share/gnome/help, so this package should own it.
Reference:
https://docs.fedoraproject.org/en-US/packaging-guidelines/#_file_and_directory_ownership
Since the help files use gnome-help
[!]: Package must own all directories that it creates.
Note: Directories without known owners: /usr/share/gnome/help/diffuse,
/usr/share/gnome/help/diffuse/ru, /usr/share/gnome/help/diffuse/C,
/usr/share/gnome/help, /usr/share/gnome/help/diffuse/cs,
/usr/share/omf/diffuse, /usr/share/gnome/help/diffuse/it
^
See point above.
[x]: Package contains no bundled libraries without FPC exception.
[x]: Changelog in prescribed format.
^
Looks fine. I'd add a blank like after each changelog entry for readability.
[x]: Sources contain only permissible code or content.
[-]: Development files must be in a -devel package
[x]: Package uses nothing in %doc for runtime.
[x]: The spec file handles locales properly.
[!]: Package consistently uses macros (instead of hard-coded directory
names).
^
You can use %{name} in the files section etc. too
[x]: Package is named according to the Package Naming Guidelines.
[x]: Package does not generate any conflict.
[x]: Package obeys FHS, except libexecdir and /usr/target.
[-]: If the package is a rename of another package, proper Obsoletes and
Provides are present.
[x]: Requires correct, justified where necessary.
[x]: Spec file is legible and written in American English.
[-]: Package contains systemd file(s) if in need.
[x]: Package is not known to require an ExcludeArch tag.
[x]: Large documentation must go in a -doc subpackage. Large could be size
(~1MB) or number of files.
Note: Documentation size is 40960 bytes in 4 files.
[!]: Package complies to the Packaging Guidelines
^
Some work needed :)
[x]: Package successfully compiles and builds into binary rpms on at least
one supported primary architecture.
[x]: Package installs properly.
[x]: Rpmlint is run on all rpms the build produces.
Note: There are rpmlint messages (see attachment).
[x]: Package does not own files or directories owned by other packages.
[x]: Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at
[Bug 1923830] Review Request: Diffuse - Diff Utility (Re-introducing Retired Package)
https://bugzilla.redhat.com/show_bug.cgi?id=1923830 --- Comment #9 from Ankur Sinha (FranciscoD) --- Hrm, fedora-review still isn't finding it. Let's try again: Spec: https://gist.githubusercontent.com/bongochong/2caa25a6121487a270ac406f2e052921/raw/e7fd871f9dc26c77c9d0844b06da9af969ff90f6/diffuse.spec SRPM: https://download.copr.fedorainfracloud.org/results/niohiani/Diffuse-Python-3/fedora-rawhide-x86_64/01963703-diffuse/diffuse-0.6.0-61.fc35.src.rpm -- 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 ___ package-review mailing list -- package-review@lists.fedoraproject.org To unsubscribe send an email to package-review-le...@lists.fedoraproject.org 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/package-review@lists.fedoraproject.org Do not reply to spam on the list, report it: https://pagure.io/fedora-infrastructure
[Bug 1923830] Review Request: Diffuse - Diff Utility (Re-introducing Retired Package)
https://bugzilla.redhat.com/show_bug.cgi?id=1923830 --- Comment #8 from niohiani --- (In reply to Ankur Sinha (FranciscoD) from comment #7) > Hello, > > Sorry, missed the notification here. Could you please post the URLs for both > the spec/srpm so that I can run it through fedora-review again? > > Cheers, > Ankur Sorry, it has been an insane past few weeks, but here you go! Spec: https://gist.github.com/bongochong/2caa25a6121487a270ac406f2e052921 SRPM: https://download.copr.fedorainfracloud.org/results/niohiani/Diffuse-Python-3/fedora-rawhide-x86_64/01963703-diffuse/diffuse-0.6.0-61.fc35.src.rpm After another round of feedback I'm sure I'll get it down pat. Apologies for the delay. -- 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 ___ package-review mailing list -- package-review@lists.fedoraproject.org To unsubscribe send an email to package-review-le...@lists.fedoraproject.org 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/package-review@lists.fedoraproject.org Do not reply to spam on the list, report it: https://pagure.io/fedora-infrastructure
[Bug 1923830] Review Request: Diffuse - Diff Utility (Re-introducing Retired Package)
https://bugzilla.redhat.com/show_bug.cgi?id=1923830 --- Comment #7 from Ankur Sinha (FranciscoD) --- Hello, Sorry, missed the notification here. Could you please post the URLs for both the spec/srpm so that I can run it through fedora-review again? Cheers, Ankur -- 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 ___ package-review mailing list -- package-review@lists.fedoraproject.org To unsubscribe send an email to package-review-le...@lists.fedoraproject.org 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/package-review@lists.fedoraproject.org Do not reply to spam on the list, report it: https://pagure.io/fedora-infrastructure
[Bug 1923830] Review Request: Diffuse - Diff Utility (Re-introducing Retired Package)
https://bugzilla.redhat.com/show_bug.cgi?id=1923830 --- Comment #6 from niohiani --- Hey again folks! Just an update. The spec file I'm amending to meet the criteria for inclusion is here now: https://gist.github.com/bongochong/2caa25a6121487a270ac406f2e052921 I think I've addressed all - or at least most - of the issues, but if I notice anything else (or you notice anything else) that needs changing, just give me a heads up. Waiting for Ankur's feedback in particular. Thanks for being so thorough and communicative. -- 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 ___ package-review mailing list -- package-review@lists.fedoraproject.org To unsubscribe send an email to package-review-le...@lists.fedoraproject.org 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/package-review@lists.fedoraproject.org Do not reply to spam on the list, report it: https://pagure.io/fedora-infrastructure
[Bug 1923830] Review Request: Diffuse - Diff Utility (Re-introducing Retired Package)
https://bugzilla.redhat.com/show_bug.cgi?id=1923830 --- Comment #5 from niohiani --- Starting to make changes now! Sorry for the delay. Long trip. Will share a modified spec file when I think I have everything in order. Thanks again! -- 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 ___ package-review mailing list -- package-review@lists.fedoraproject.org To unsubscribe send an email to package-review-le...@lists.fedoraproject.org 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/package-review@lists.fedoraproject.org Do not reply to spam on the list, report it: https://pagure.io/fedora-infrastructure
[Bug 1923830] Review Request: Diffuse - Diff Utility (Re-introducing Retired Package)
https://bugzilla.redhat.com/show_bug.cgi?id=1923830 --- Comment #4 from niohiani --- Thank you for all of the useful input. I'll look over all of this and start making the necessary changes. I'm pretty sure I can slim down the Build Requirements in the spec too. It might take a few days for me to have *everything* remedied, as I have to do some pretty heavy travelling this week, but I will get it done. -- 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 ___ package-review mailing list -- package-review@lists.fedoraproject.org To unsubscribe send an email to package-review-le...@lists.fedoraproject.org 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/package-review@lists.fedoraproject.org
[Bug 1923830] Review Request: Diffuse - Diff Utility (Re-introducing Retired Package)
https://bugzilla.redhat.com/show_bug.cgi?id=1923830
--- Comment #3 from Ankur Sinha (FranciscoD) ---
Looks pretty good. Here is a first round of reviews. A few blockers here that
need fixing:
Package Review
==
Legend:
[x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated
[ ] = Manual review needed
Issues:
===
- Package uses either %{buildroot} or $RPM_BUILD_ROOT
Note: Using both %{buildroot} and $RPM_BUILD_ROOT
See: https://docs.fedoraproject.org/en-US/packaging-guidelines/#_macros
- If (and only if) the source package includes the text of the license(s)
in its own file, then that file, containing the text of the license(s)
for the package is included in %license.
Note: License file COPYING is not marked as %license
See: https://docs.fedoraproject.org/en-US/packaging-
guidelines/LicensingGuidelines/#_license_text
- Package does not use a name that already exists.
Note: A package with this name already exists. Please check
https://src.fedoraproject.org/rpms/diffuse
See: https://docs.fedoraproject.org/en-US/packaging-
guidelines/Naming/#_conflicting_package_names
^
This is OK.
- Spec file name must match the spec package %{name}, in the format
%{name}.spec.
Note: diffuse3-meson.spec should be diffuse.spec
See: https://docs.fedoraproject.org/en-US/packaging-
guidelines/#_spec_file_naming
- Why is the release 60? It can start with 1
- For the SourceURL, please refer to this page:
https://docs.fedoraproject.org/en-US/packaging-guidelines/SourceURL/#_git_hosting_services
- You do not need to BR: python3, python3-devel will pull it in.
- You do not need to BR: /usr/bin/python either.
- You do not need to Require: python3 either
- Are the explicit provides necessary?
- If this is a pure python package, it should be use: BuildArch: noarch, and
then you do not need to explicitly disable the debug package. If not, it must
generate debuginfo.
https://docs.fedoraproject.org/en-US/packaging-guidelines/Debuginfo/
- We should use the metainfo file for appdata now:
https://docs.fedoraproject.org/en-US/packaging-guidelines/AppData/
- Please also remember to validate the appdata file in the check section as
listed there
- the %files tag is used twice, you can remove the first one.
= MUST items =
Generic:
[x]: Package is licensed with an open-source compatible license and meets
other legal requirements as defined in the legal section of Packaging
Guidelines.
[x]: License field in the package spec file matches the actual license.
Note: There is no build directory. Running licensecheck on vanilla
upstream sources. Licenses found: "Unknown or generated", "GNU General
Public License, Version 2", "*No copyright* GNU General Public
License, Version 2". 101 files have unknown license. Detailed output
of licensecheck in /home/asinha/dump/fedora-
reviews/1923830-diffuse3-meson/licensecheck.txt
[!]: Package requires other packages for directories it uses.
Note: No known owner of /usr/share/diffuse
^
Perhaps the file list should have %{_datadir}/%{name} (remove the /*)?
[!]: Package must own all directories that it creates.
Note: Directories without known owners: /usr/share/gnome/help,
/usr/share/diffuse
^
I think you need to own the /usr/share/gnome/help directory (while you do not
need to Require a package for it). Take a look here:
https://docs.fedoraproject.org/en-US/packaging-guidelines/#_the_directory_is_owned_by_a_package_which_is_not_required_for_your_package_to_function
[!]: %build honors applicable compiler flags or justifies otherwise.
^
Is nothing being built in the build step? You can remove %meson_build if that's
the case.
[x]: Package contains no bundled libraries without FPC exception.
[!]: Changelog in prescribed format.
^
Please correct the changelog. It should be version-release at the end, and the
entries are generally short points. You can give more verbose explanations in
the git commit messages when committing to Fedora SCM
https://docs.fedoraproject.org/en-US/packaging-guidelines/#changelogs
[x]: Sources contain only permissible code or content.
[-]: Development files must be in a -devel package
[x]: Package uses nothing in %doc for runtime.
[x]: The spec file handles locales properly.
[!]: Package consistently uses macros (instead of hard-coded directory
names).
^
Please use %{buildroot} consistently (and not mix $RPM_BUILD_ROOT too).
[!]: Package is named according to the Package Naming Guidelines.
^
Please see my comment above.
[x]: Package does not generate any conflict.
[x]: Package obeys FHS, except libexecdir and /usr/target.
[-]: If the package is a rename of another package, proper Obsoletes and
Provides are present.
[!]: Requires correct, justified where necessary.
^
Please see my comment on the extra Requires.
[x]: Spec file is legible and written in American English.
[-]: Package contains systemd file(s) if in need.
[!]: Useful -debuginfo pac
[Bug 1923830] Review Request: Diffuse - Diff Utility (Re-introducing Retired Package)
https://bugzilla.redhat.com/show_bug.cgi?id=1923830 --- Comment #2 from Ankur Sinha (FranciscoD) --- A few notes already: - We use the `fedora-review` tool for the first round of reviews, and that downloads the spec/srpm from the bug here, so we need to make sure that the links we provide here are ones that can be directly downloaded from: Spec: https://download.copr.fedorainfracloud.org/results/niohiani/Diffuse-Python-3/fedora-rawhide-x86_64/01817739-diffuse/diffuse3-meson.spec SRPM: https://copr-be.cloud.fedoraproject.org/results/niohiani/Diffuse-Python-3/srpm-builds/01817739/diffuse-0.6.0-60.fc33.src.rpm - your spec file must match the name of the src.rpm (which both follow the naming guidelines: https://docs.fedoraproject.org/en-US/packaging-guidelines/Naming/). So the spec should be called either python-diffuse or diffuse, depending on whether it's a library or only an application: and https://docs.fedoraproject.org/en-US/packaging-guidelines/Python/#_naming -- 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 ___ package-review mailing list -- package-review@lists.fedoraproject.org To unsubscribe send an email to package-review-le...@lists.fedoraproject.org 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/package-review@lists.fedoraproject.org
[Bug 1923830] Review Request: Diffuse - Diff Utility (Re-introducing Retired Package)
https://bugzilla.redhat.com/show_bug.cgi?id=1923830 Ankur Sinha (FranciscoD) changed: What|Removed |Added Status|NEW |ASSIGNED CC||sanjay.an...@gmail.com Assignee|nob...@fedoraproject.org|sanjay.an...@gmail.com Doc Type|--- |If docs needed, set a value Flags||fedora-review? --- Comment #1 from Ankur Sinha (FranciscoD) --- Heya, I'll review this one. Could you review a few other tickets in the meantime also please? You can find pending ones here: https://fedoraproject.org/PackageReviewStatus/ Cheers, Ankur -- 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 ___ package-review mailing list -- package-review@lists.fedoraproject.org To unsubscribe send an email to package-review-le...@lists.fedoraproject.org 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/package-review@lists.fedoraproject.org
