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

[Server 3.0] now needs a qApp #3528

Merged
merged 6 commits into from
Sep 27, 2016
Merged

Conversation

elpaso
Copy link
Contributor

@elpaso elpaso commented Sep 25, 2016

DO NOT MERGE!
READY TO MERGE

All the tests need to be adapted to Py3

@elpaso elpaso added Prototype For comment only, do not merge. Avoid using this tag and open a QEP instead! Requires Tests! Waiting on the submitter to add unit tests before eligible for merging API Break! Breaks stable API. Proceed with extreme caution!! labels Sep 25, 2016
@elpaso
Copy link
Contributor Author

elpaso commented Sep 25, 2016

Initial fix for #3528
#3510

@nyalldawson
Copy link
Collaborator

@elpaso This looks great, and I'm really glad to see someone resurrecting these tests! Just a note (you may already be aware of this or not) but to get the tests to run on Travis you'll need to modify the blacklist (https://github.com/qgis/QGIS/blob/master/ci/travis/linux/qt5/blacklist.txt)

@sbrunner
Copy link
Contributor

Initial fix for ...

It point himself ...

@elpaso
Copy link
Contributor Author

elpaso commented Sep 26, 2016

@sbrunner bad cut/paste here is the link I wanted to add: #3510

@sbrunner
Copy link
Contributor

@elpaso thanks :-)

@elpaso elpaso removed the Requires Tests! Waiting on the submitter to add unit tests before eligible for merging label Sep 26, 2016
@elpaso
Copy link
Contributor Author

elpaso commented Sep 26, 2016

@sbrunner plase have have a look to the AC tests, I didn't check them, just quickly added the QgsApplication app creation which is now required.

@nyalldawson
Copy link
Collaborator

@elpaso I think it's just missing the QgsApplication import. Everything else looks good to me.

@elpaso elpaso removed API Break! Breaks stable API. Proceed with extreme caution!! Prototype For comment only, do not merge. Avoid using this tag and open a QEP instead! labels Sep 27, 2016
@elpaso elpaso merged commit 6b7d7b0 into qgis:master Sep 27, 2016
@elpaso elpaso deleted the server-needs-qgsapp branch September 27, 2016 07:04
@sbrunner
Copy link
Contributor

Tanks @elpaso good job :-)

@dmarteau
Copy link
Contributor

Hi

@elpaso aeb5ff2 broke python bindings by making init() private:

/Users/david/Projets/qgis/QGIS/build/python/server/sip_serverpart1.cpp:42:21: error: 'init' is a private member of
      'QgsServer'
            sipCpp->init();
                    ^
/Users/david/Projets/qgis/QGIS/python/../src/server/qgsserver.h:88:17: note: declared private here
    static bool init();

@elpaso
Copy link
Contributor Author

elpaso commented Sep 27, 2016

Did you run make clean ?

@dmarteau
Copy link
Contributor

My bad,

make clean solve the issue.

@elpaso
Copy link
Contributor Author

elpaso commented Sep 27, 2016

not completely yours: we had for ages something wrong in cmake or in the cmake config that does not rebuild sip bindings if the sip source files change, at least in some cases. If you could fix it it would be awesome.

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.

None yet

4 participants