Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Remove "the"-prefix from method signatures
- Loading branch information
Showing
3 changed files
with
41 additions
and
204 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
382b213
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are tons of it. Is it something wanted to remove all of them?
382b213
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd prefer to have them removed, but no particular hurry.
I'm not sure if it accounts for an API change, I never found a reason why it should, but @nyalldawson once pointed out that it is... (basically this determines if it's required to do this before QGIS 3)
382b213
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't it potentially a break if someone's using ?
I know that some people strictly always use named arguments in python code like this...
382b213
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That was what I suspected when you told me it's public API... But using these as named arguments didn't work for me, does it for you? Maybe a sip compile option or version issue?
382b213
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah... you're right. I'd always just assumed that worked!! So I guess no break here then.
382b213
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
... which would be really nice ...
Thanks, in this case no particular hurry to drop these prefixes, I'll just address them as I bump into them.