search

lxqt / libdbusmenu-lxqt

Other
2 stars 3 forks source link

Check for doxygen if option enabled #7

Closed stefonarch closed 7 months ago

stefonarch commented 9 months ago

Fixes https://github.com/lxqt/libdbusmenu-lxqt/issues/6

tsujan commented 9 months ago

This is good, of course.

But, IMHO, we need to decide whether we want to compile the doc by default, or even if we want it at all. Ordinary users don't need it, and devs usually consult the code and the explanatory comments of public methods in headers.

stefonarch commented 9 months ago

I'd say to remove all of that, on NEON unstable (ubuntu) with this option enabled build fails with -- No package 'QJson' found and there is no libjson-dev nowhere.

tsujan commented 9 months ago

I agree.

@yan12125?

yan12125 commented 9 months ago

I'm fine with removing Doxygen support. Although I believe documentation is useful developers, this library may be used in LXQt only.

-- No package 'QJson' found

That seems more related to tests rather than docs https://github.com/search?q=repo%3Alxqt%2Flibdbusmenu-lxqt%20qjson&type=code

tsujan commented 9 months ago

I believe documentation is useful developers,

I may be wrong, but I found nothing in them more informative than what is in /usr/include/dbusmenu-lxqt.

yan12125 commented 9 months ago

I believe documentation is useful developers,

I may be wrong, but I found nothing in them more informative than what is in /usr/include/dbusmenu-lxqt.

Yes, generated documentation is unlikely more informative. Personally I feel them easier to read. Anyway, that's irrelevant as this library is unlikely to be used outside LXQt.

stefonarch commented 8 months ago

Ran again into it, with the build script which should add this repo. Shall we just merge it for the moment?

tsujan commented 8 months ago

Shall we just merge it for the moment?

Do it if it solves your problem. But please remind me to remove the doc before the next release.