Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Header warning fixes and build cleanup #5

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

sgaist
Copy link

@sgaist sgaist commented Jul 4, 2016

This pull request fixes the public headers include warnings generated when compiling the module as well as a private header missing "warning" warning.

It also cleans up the module build setup.

As a side effect, that makes the module build on Windows properly with VS2015.

@VSRonin
Copy link
Owner

VSRonin commented Jul 5, 2016

I'm not completely sure about this. The whole thing is deployed to work as a Qt Module, make install does that.
QT += xlsx
Does the include and linking work for you with no include warnings.

I personally don't like to have external libraries work this way but it's the original author choice

@sgaist
Copy link
Author

sgaist commented Jul 5, 2016

If you mean, can I build an application without warning ? Yes.

What I fixed with this PR is a series of warning generated when building a Qt module with "unclean" headers.

In the absolute, the standard Qt public headers are also written differently when using classes from other submodules (i.e. <QtModule/qclassname.h>) but I wanted to let that for another request.

This simplifies the life of people not using qmake to handle their project.

Side effect: fixes building with MSVC 2015
@hongnod
Copy link

hongnod commented Jul 5, 2016

I made a shared lib fork--https://github.com/topillar/QtXlsxWriter, I don't like module way either, but it hard to follow other's commits.

@sgaist sgaist changed the title Header warning fixes Header warning fixes and build cleanup Jul 5, 2016
rbulygin pushed a commit to rbulygin/QtXlsxWriter that referenced this pull request Jun 29, 2017
Synchronization with origin
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.

3 participants