Pushing code for review
-
Hi,
It depends if it can be considered a new feature or a bug fix. If a new feature you should post against dev. You can't break the repo. Gerrit is the first step, you get your code reviewed there, and only if approved and passing the CI, you'll get it included in Qt.
As for your utility class, you should do it in a separate commit to keep the atomicity of your work unless it's needed for your first patch.
@SGaist
Hello, thanks for the prompt reply.
Since my changes are at the low-level end of Qt, and currently only for X11, I don't need my utility class for them. The utility is just a simple wrapper that uses them, nothing more. I consider it a new feature since it's just an additional virtual function added to theQPlatformWindowclass and doesn't depend on anything else, and nothing depends on it as well. If I may one additional question:
Should I first make a feature request, and then submit my code as a possible "solution" or just pushing it is fine?Kind regards.
-
I'd make the feature request first if there's nothing already in the bug tracker. It will add visibility for people searching for something similar.
-
I'd make the feature request first if there's nothing already in the bug tracker. It will add visibility for people searching for something similar.
@SGaist
Ok, thank you! -
-
Nothing wrong with that. The commits in Qt base cover QPA as well as core, xml, network etc. There's no special order or classification for that. You can have a bug fix for OS X followed by the implementation of a new feature of one of the core class.
The parent of your commit will depend on the state of the repository when you do your commit and send it for review.
You should add reviewers to your submission for it to go forward
-
Nothing wrong with that. The commits in Qt base cover QPA as well as core, xml, network etc. There's no special order or classification for that. You can have a bug fix for OS X followed by the implementation of a new feature of one of the core class.
The parent of your commit will depend on the state of the repository when you do your commit and send it for review.
You should add reviewers to your submission for it to go forward
The parent of your commit will depend on the state of the repository when you do your commit and send it for review.
So I guess then that my dependence of the iOS changes is just that. This explains it, thanks.
You should add reviewers to your submission for it to go forward
Since I don't know anyone over there I wouldn't go around adding random people for reviewers, would I? How should I proceed with that instead?
-
You should look in the git history of the files that you modified and check who reviewed the last works on them, that should give you a starting point. You can also add the current maintainer of the Qt module/QPA plugin.
-
You should look in the git history of the files that you modified and check who reviewed the last works on them, that should give you a starting point. You can also add the current maintainer of the Qt module/QPA plugin.
@SGaist
Ok, I'll do that. Thank you! -
You should look in the git history of the files that you modified and check who reviewed the last works on them, that should give you a starting point. You can also add the current maintainer of the Qt module/QPA plugin.
@SGaist
Hello,
I'm sorry to bother you again with this, but naturally I got a -1 on my code. The person requested I add autotest and documentation. Could you point me to a resource where I could see how to do that (the testing part mostly)? I think I'll be able to muddle through the documentation issue on my own. I saw that with creator I can create tests but it seems it requires a commercial license, which I do not own ... any suggestions?
Additionally, how should I commit my new changes? Should I (soft) reset the HEAD, put the corrections in, and then commit again, or just make the changes without resetting the HEAD and commit directly?Kind regards.
-
Nop, you don't need any commercial license for that.
Usually when I add a new test, I first check if there's already a test or unit test covering the class / function I'm modifying and update it as needed.
If there's none or when adding a new class, I usually copy a small unit test so I already have the base for coding style there as well as the license and copyright notice.
When you add a new class/file, set the copyright as yours (the license doesn't change, but it's your work so you have the copyright)
-
Nop, you don't need any commercial license for that.
Usually when I add a new test, I first check if there's already a test or unit test covering the class / function I'm modifying and update it as needed.
If there's none or when adding a new class, I usually copy a small unit test so I already have the base for coding style there as well as the license and copyright notice.
When you add a new class/file, set the copyright as yours (the license doesn't change, but it's your work so you have the copyright)
@SGaist
Right, thanks, what about committing? I want to be sure that I get patch set 2, 3 etc, instead of some new patch set.Kind regards.
-
Sorry, I knew I was forgetting something…
If you are updating your last commit just amend it, add your modifications/commit message update and push it again as is. The ID will be used to update your patch set
-
Sorry, I knew I was forgetting something…
If you are updating your last commit just amend it, add your modifications/commit message update and push it again as is. The ID will be used to update your patch set
@SGaist
Ok. Thank you!
12/15