Skip to content

Conversation

@slipher
Copy link
Contributor

@slipher slipher commented Nov 8, 2025

Pull Request check-list

Please make sure to review and check all of these items:

  • Does the code keep building with this change?
    It's closer to building than before at least.
  • Do the unit tests pass with this change?
  • Is the commit message formatted according to CONTRIBUTING.MD?
  • If this change fixes a bug (or a performance problem), is a regression
    test (or a benchmark) included?

Description of change

Fix various build issues I encountered while trying to update the library:

  • The autogen code didn't like .cpp files being in a subdir
  • Missing QML files
  • Icons not available with FLUID_INSTALL_ICONS=0
  • Units singleton not instantiated

@plfiorini
Copy link
Member

Why revert the commit 177b5a8 that is not a desirable fix.
Not sure what build issue there was though, because the CI was building.
However I just pushed a few commits made in the last days, can you check with those to see if it builds fine?

@plfiorini
Copy link
Member

Some commits in this merge request can be accepted though, for example the singleton and the icons ones.

@plfiorini plfiorini self-requested a review November 8, 2025 15:07
@plfiorini plfiorini self-assigned this Nov 8, 2025
@slipher
Copy link
Contributor Author

slipher commented Nov 8, 2025

Not sure what build issue there was though, because the CI was building.

An auto-generated .cpp file wasn't compiling. Luckily this has been fixed by one of the new commits that renames the files again.

Remaining 3 commits are now rebased.

> Note: set_source_files_properties needs to be called before qt_add_qml_module
-- https://doc.qt.io/qt-6/qml-singleton.html

Also file path was wrong.
- Add back the liri.io prefix for embedded icon paths (it was dropped when
  deleting icons.qrc)
- Produce the correct path in iconUrl()
- Use Q_INIT_RESOURCE so that the linker doesn't trash the resources
  when building a static lib
@plfiorini plfiorini merged commit ec320c7 into lirios:develop Nov 14, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants