Bug 283847 - textproc/zxing-cpp: Update to 2.3.0
Summary: textproc/zxing-cpp: Update to 2.3.0
Status: Open
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Only Me
Assignee: freebsd-kde (group)
URL: https://github.com/zxing-cpp/zxing-cp...
Keywords:
Depends on:
Blocks:
 
Reported: 2025-01-04 16:16 UTC by Daniel Engberg
Modified: 2025-01-05 09:54 UTC (History)
1 user (show)

See Also:
jhale: maintainer-feedback+


Attachments
Patch for zxing-cpp (4.01 KB, patch)
2025-01-04 16:16 UTC, Daniel Engberg
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Daniel Engberg freebsd_committer freebsd_triage 2025-01-04 16:16:35 UTC
Created attachment 256415 [details]
Patch for zxing-cpp

* Enable unit tests
* Remove unnecessary patch
* Clean up port Makefile

Compile and runtime tested on FreeBSD 14.2-RELEASE (amd64) (make, make check-plist, make test)

Poudriere testport OK 13.4-RELEASE (amd64)
Poudriere testport OK 14.2-RELEASE (amd64)

Tested with following consumers in 13.4-RELEASE and 14.2-RELEASE (amd64) using Poudriere:
deskutils/kodaskanna
editors/libreoffice
graphics/kf5-prison
graphics/kf6-prison
net/kitinerary
net-im/kaidan
textproc/gstreamer1-plugins-zxing
Comment 1 Jason E. Hale freebsd_committer freebsd_triage 2025-01-05 09:12:18 UTC
Please try to reasonably comply with the corrections given by portclippy(1) and portlint(1), especially for ports maintained by teams unless otherwise specified. Some tooling decisions are dumb and should be ignored and/or corrected, but when multiple people are working on the same port, agreeing with the dumb tool might be the better option as apposed to unnecessary repo churn between updates until said tool is brought up to speed.

USE_GITHUB clearly belongs in the USE_ block. No reason to change this. While I can see a case for moving it to a separate block if GH_ variables are also defined, this is not true here. Please revert this line's position in the Makefile.

CMAKE_TESTING_ON shouldn't be a separate block. It should be part of the CMAKE_ block, but should be listed towards the end, because testing is done at the end of the process. portclippy(1) actually gets this right. Whitespace alignment within that block would be needed, but that change would be preferable to a separate block.

Why nuke the useful comments for the CMAKE_OFF variable in the Makefile, either? Again, this is a team port, so verbosity within the Makefile and patches is an absolute virtue.
Comment 2 Daniel Engberg freebsd_committer freebsd_triage 2025-01-05 09:54:41 UTC
(In reply to Jason E. Hale from comment #1)
They're tools not rules and Porters Handbook also mentions that they're not to be treated as such. Some suggestions just makes things akward/hard to read/follow and isn't followed overall in the tree due to that. Such as not separating USE_GITHUB which is done  all the time so changed consistency with the rest of the tree. Feel free to revert whatever changes you disagree upon.

While out of scope for the PR placement suggestions of CONFIGURE_* CMAKE_* MESON_* is also one thing that we don't adhere to what our tools suggests because of convenience and ease of reviewing. While not in this case these are more often than not placed last before defining menu options as these often are related to variables in some way or another while tooling wants these sections in alphabetical order which results in a lot jumping back and forth in the text. I have yet to find someone who objects to this layout. This also why CMAKE_TESTING* isn't grouped together in many cases.

Comments are unnecessary now that we have a working unit test? People also seem to prefer that comments are left in commit message rather than cluttering the Makefile?