Skip to content

Commit

Permalink
fix windows build of QgsFeatureRequest::OrderBy again (reapply 8f29f28
Browse files Browse the repository at this point in the history
…and 3c843a8)
  • Loading branch information
jef-n committed May 19, 2017
1 parent 0923f56 commit 1d5d92e
Showing 1 changed file with 8 additions and 8 deletions.
16 changes: 8 additions & 8 deletions src/core/qgsfeaturerequest.h
Original file line number Diff line number Diff line change
Expand Up @@ -215,49 +215,49 @@ class CORE_EXPORT QgsFeatureRequest
*
* \since QGIS 2.14
*/
class CORE_EXPORT OrderBy : public QList<QgsFeatureRequest::OrderByClause>
class OrderBy : public QList<QgsFeatureRequest::OrderByClause>
{
public:

/**
* Create a new empty order by
*/
OrderBy()
CORE_EXPORT OrderBy()
: QList<QgsFeatureRequest::OrderByClause>()
{}

/**
* Create a new order by from a list of clauses
*/
OrderBy( const QList<QgsFeatureRequest::OrderByClause> &other );
CORE_EXPORT OrderBy( const QList<QgsFeatureRequest::OrderByClause> &other );

/**
* Get a copy as a list of OrderByClauses
*
* This is only required in Python where the inheritance
* is not properly propagated and this makes it usable.
*/
QList<QgsFeatureRequest::OrderByClause> list() const;
QList<QgsFeatureRequest::OrderByClause> CORE_EXPORT list() const;

/**
* Serialize to XML
*/
void save( QDomElement &elem ) const;
void CORE_EXPORT save( QDomElement &elem ) const;

/**
* Deserialize from XML
*/
void load( const QDomElement &elem );
void CORE_EXPORT load( const QDomElement &elem );

/**
* Returns a set of used attributes
*/
QSet<QString> usedAttributes() const;
QSet<QString> CORE_EXPORT usedAttributes() const;

/**
* Dumps the content to an SQL equivalent syntax
*/
QString dump() const;
QString CORE_EXPORT dump() const;
};

/**
Expand Down

13 comments on commit 1d5d92e

@NathanW2
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So why does each method need a CORE_EXPORT and not just the class?

@nyalldawson
Copy link
Collaborator

@nyalldawson nyalldawson commented on 1d5d92e May 19, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because otherwise the Windows build breaks ;)

But seriously... I'm not sure. It's something to do with inheriting QList<>... Without this change the build complains about a bunch of unimplemented methods like equality operators and hashes. None of these are actually required though.

@NathanW2
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, that is what I don't quite understand. I was going to fix it last night but thought it didn't make sense.

@nyalldawson
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Incidentally... this commit has now been applied 3 times :)

8f29f28
3c843a8
and this one...

@3nids
Copy link
Member

@3nids 3nids commented on 1d5d92e May 19, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

which has been again reverted b0bb873

@m-kuhn
Copy link
Member

@m-kuhn m-kuhn commented on 1d5d92e May 19, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@3nids should we temporarily sip blacklist this file?

@jef-n
Copy link
Member Author

@jef-n jef-n commented on 1d5d92e May 19, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sipify.pl now removes _EXPORT from the method declarations and doesn't complain about the missing EXPORT at class level if there method exports.

@m-kuhn
Copy link
Member

@m-kuhn m-kuhn commented on 1d5d92e May 19, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks @jef-n!

@nyalldawson
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jef-n personally I would have preferred to see a cron job running on the qgis server which reapplied this commit just before the daily osgeo4w builds, and then reverted it immediately after so that Travis failures are fixed. Can you please rework?

;)

@NathanW2
Copy link
Member

@NathanW2 NathanW2 commented on 1d5d92e May 20, 2017 via email

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nyalldawson
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Corporate shills like you @NathanW2

@NathanW2
Copy link
Member

@NathanW2 NathanW2 commented on 1d5d92e May 20, 2017 via email

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jef-n
Copy link
Member Author

@jef-n jef-n commented on 1d5d92e May 20, 2017 via email

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.