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



--- Comment #9 from Cristian Le <[email protected]> ---
> Source0: %{name}-%{archive_suffix}.zip

The source is quite ambiguous where it comes from. Could maybe use `%forgemeta`
instead, openmw is an example for how to define it with git submodules, and the
generate-source.sh can just become a scriptlet that updates the git submodule
references? If not at least put a comment pointing to the upstream source in
the spec file


> Patch0: %{name}--system-openal.patch

Providence of patches please?

> While stb is unbundled, it's a static library, so it's still included in the 
> License tag.

Well, so is `nlohmann_json` right?

> %files data

Same license comment as `davegnukem`

> %global httplib_bundled 0

Can make this a %bcond instead?

> %global rapidjson_cmake OFF

(no longer applies, but fyi) CMake works fine with `0`/`1` from `%with_`
macros. You might want to add `:BOOL`, but don't remember if it affects
normalization.

> -DCMAKE_POLICY_VERSION_MINIMUM=3.5

Can raise the issue to upstream?

> sed -e 's|^#include "3rdparty/stb/|#include "stb/|g' -i 
> src/augs/image/image.cpp

This looks like something that can be fixed in upstream by moving the
`target_include_directories` one layer up. Can propose to upstream?

> -DARCHITECTURE=native

Oh boy that flag does some vodoo. Can you check [1] to see if you should
conditionalize it?

> -DBUILD_UNIT_TESTS=OFF

Can add a comment on why?

> Generate a .desktop file

Can make it a static template file like the `wrapper.sh` file?

> # %%cmake_install doesn't do anything, so we gotta copy stuff manually.

Haha. Can ask them to do something :). Make sure that you do RPATH reset if you
install manually. Alternatively, what's the deal with
cmake/steam_integration/CMakeLists.txt?

> BSD-2-Clause

Why it was added is not explained. I did find
`src/3rdparty/yojimbo/sodium/sodium_hash_sha512_cp.c` from license-check. Also
going through the `src/3rdparty` a few others are not covered or deleted:
crc32, glad, glfw, rapidjson (missing similar comment to cpp-httplib)

From last rpmlint:

> hypersomnia-data.noarch: W: dangling-relative-symlink 
> /usr/share/hypersomnia/content/fonts/Ubuntu-Regular.ttf 
> ../../../fonts/liberation-sans-fonts/LiberationSans-Regular.ttf

Need to re-run the fedora-review on the newer one, will do that after the next
reply

[1]:
https://github.com/TeamHypersomnia/Hypersomnia/commit/9cdc28ae2bbf6b5a2b4cddad89feadcbbeed4734


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

Report this comment as SPAM: 
https://bugzilla.redhat.com/enter_bug.cgi?product=Bugzilla&format=report-spam&short_desc=Report%20of%20Bug%202390526%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