search

dcourtois / premake-qt

Premake module adding support for Qt to actions (Visual Studio, makefiles, etc.)
Do What The F*ck You Want To Public License
45 stars 11 forks source link

Add test project. #21

Closed Jarod42 closed 2 years ago

Jarod42 commented 2 years ago

Add test project (with moc, qrc and uic) with workflow and badge. (see actions' results). I also provide premake5-without-premake-qt.lua which does the same thing, but without your module (using rules for .ui).

I will try to test/fix other generators in my premake testing repo.

gmake2 doesn't support premake-qt :-/ Other generators add the new rules :-) but need some tweaks (escape/quote, duplicated buildinputs, ...).

I didn't succeed yet to have working Qt6 on ubuntu neither.

dcourtois commented 2 years ago

Hi,

I'm sorry but in its current state, I won't merge this. The main reason is I really don't feel comfortable adding all those tests and CI stuff into this repository. Especially when there are so few comments. Don't forget that if I decide to merge this, I will be responsible to maintain this in the future, which I'm not really willing to do, unless there is a really good reason to.

The other reason why I won't merge this is that it doesn't fix anything or doesn't improve the existing. I saw that you have issues with the gmake2 action, and any pull request actually fixing this action would be greatly appreciated (and merged, provided it's following the code convention of the rest of the addon, and is commented/documented) And maybe a very simple non-regression test for that commit would be ok, but not a general test and CI pull requst like that.

I hope you understand.

(I'm leaving this pull request open though, in case you want to update and force push the branch to be a fix for gmake2 or the other issues you mentioned on the other generators)

Regards, Damien.

Jarod42 commented 2 years ago

The main reason is I really don't feel comfortable adding all those tests and CI stuff

Tests without your module can be removed (so file premake-without-premake-qt, and associated yml file can be removed). (I added them partially to show alternative for .ui files (and as I first fail to use your module with another generator)).

sh script to test the project can also be simplified (I mostly copy the one of my testing project where I test all generators) and possibly directly be put in yml.

Testing project (cpp part) might be simplified, but I already tried to have a minimal one.

there are so few comments

I can add some if needed.

it doesn't fix anything or doesn't improve the existing.

CI proves that the addon is (at least partially) working :-) and show a working project (so provide implicit documentation).

I'm leaving this pull request open You can close the PR if you don't want tests/CI (I'm open to change the tests/CI to be more compatible with your vision)

For additional fix/improvement, I will simply create others PRs :)

I have already a local branch for usage of externalincludedirs (previously sysincludedirs) to avoid to see warning from Qt header.

I will certainly create other PRs for improvements and to fix other generators. But I have to find how to have Qt on Ubuntu in CI for testing :-)

For gmake2, for me, the issue is with that action. I will see if it is "easily" fixable in premake5 directly.

Anyway, I think I will put equivalent test in https://github.com/Jarod42/premake-sample-projects once I find how to have CI for Ubuntu with Qt (No need for mac yet, as xcode action doesn't support custom-rule).

Jarod42 commented 2 years ago

Simplification done. CI still passing: https://github.com/Jarod42/premake-qt/actions/runs/2703165335

Jarod42 commented 2 years ago

Just add tests in https://github.com/Jarod42/premake-sample-projects for premake-qt:

Working:

Pre-requirement not supported:

Partial working:

Doesn't work:

Jarod42 commented 2 years ago

UTs added in my https://github.com/Jarod42/premake-sample-projects for premake-qt. So adding equivalent UTs here might be redundant. Closed.