Skip to content

Header warning fixes and build cleanup#5

Open
sgaist wants to merge 3 commits into
VSRonin:masterfrom
sgaist:master
Open

Header warning fixes and build cleanup#5
sgaist wants to merge 3 commits into
VSRonin:masterfrom
sgaist:master

Conversation

@sgaist
Copy link
Copy Markdown

@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
Copy Markdown
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
Copy Markdown
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
Copy Markdown

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