Skip to content
  • Categories
  • Recent
  • Tags
  • Popular
  • Users
  • Groups
  • Search
  • Get Qt Extensions
  • Unsolved
Collapse
Brand Logo
  1. Home
  2. Special Interest Groups
  3. Qt Contribution
  4. Pushing code for review

Pushing code for review

Scheduled Pinned Locked Moved Solved Qt Contribution
gerritgitguiwidgetsqpa
15 Posts 2 Posters 6.8k Views
  • Oldest to Newest
  • Newest to Oldest
  • Most Votes
Reply
  • Reply as topic
Log in to reply
This topic has been deleted. Only users with topic management privileges can see it.
  • S Offline
    S Offline
    SGaist
    Lifetime Qt Champion
    wrote on 24 Dec 2015, 00:31 last edited by
    #4

    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.

    Interested in AI ? www.idiap.ch
    Please read the Qt Code of Conduct - https://forum.qt.io/topic/113070/qt-code-of-conduct

    K 1 Reply Last reply 24 Dec 2015, 00:33
    2
    • S SGaist
      24 Dec 2015, 00:31

      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.

      K Offline
      K Offline
      kshegunov
      Moderators
      wrote on 24 Dec 2015, 00:33 last edited by
      #5

      @SGaist
      Ok, thank you!

      Read and abide by the Qt Code of Conduct

      K 1 Reply Last reply 25 Dec 2015, 20:25
      1
      • K kshegunov
        24 Dec 2015, 00:33

        @SGaist
        Ok, thank you!

        K Offline
        K Offline
        kshegunov
        Moderators
        wrote on 25 Dec 2015, 20:25 last edited by kshegunov
        #6

        @SGaist
        I've submitted my code but I think I did something wrong, because my change is parented to some iOS stuff ... Perhaps, because I've cloned the dev branch and made the changes there. Is this the right way to do it, or I take a versioned branch and only push to dev?

        Read and abide by the Qt Code of Conduct

        1 Reply Last reply
        2
        • S Offline
          S Offline
          SGaist
          Lifetime Qt Champion
          wrote on 25 Dec 2015, 22:22 last edited by
          #7

          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

          Interested in AI ? www.idiap.ch
          Please read the Qt Code of Conduct - https://forum.qt.io/topic/113070/qt-code-of-conduct

          K 1 Reply Last reply 26 Dec 2015, 02:53
          2
          • S SGaist
            25 Dec 2015, 22:22

            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

            K Offline
            K Offline
            kshegunov
            Moderators
            wrote on 26 Dec 2015, 02:53 last edited by
            #8

            @SGaist

            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?

            Read and abide by the Qt Code of Conduct

            1 Reply Last reply
            2
            • S Offline
              S Offline
              SGaist
              Lifetime Qt Champion
              wrote on 26 Dec 2015, 20:30 last edited by
              #9

              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.

              Interested in AI ? www.idiap.ch
              Please read the Qt Code of Conduct - https://forum.qt.io/topic/113070/qt-code-of-conduct

              K 2 Replies Last reply 26 Dec 2015, 22:46
              3
              • S SGaist
                26 Dec 2015, 20:30

                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.

                K Offline
                K Offline
                kshegunov
                Moderators
                wrote on 26 Dec 2015, 22:46 last edited by
                #10

                @SGaist
                Ok, I'll do that. Thank you!

                Read and abide by the Qt Code of Conduct

                1 Reply Last reply
                1
                • S SGaist
                  26 Dec 2015, 20:30

                  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.

                  K Offline
                  K Offline
                  kshegunov
                  Moderators
                  wrote on 29 Dec 2015, 07:39 last edited by kshegunov
                  #11

                  @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.

                  Read and abide by the Qt Code of Conduct

                  1 Reply Last reply
                  1
                  • S Offline
                    S Offline
                    SGaist
                    Lifetime Qt Champion
                    wrote on 29 Dec 2015, 22:35 last edited by
                    #12

                    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)

                    Interested in AI ? www.idiap.ch
                    Please read the Qt Code of Conduct - https://forum.qt.io/topic/113070/qt-code-of-conduct

                    K 1 Reply Last reply 29 Dec 2015, 22:54
                    1
                    • S SGaist
                      29 Dec 2015, 22:35

                      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)

                      K Offline
                      K Offline
                      kshegunov
                      Moderators
                      wrote on 29 Dec 2015, 22:54 last edited by
                      #13

                      @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.

                      Read and abide by the Qt Code of Conduct

                      1 Reply Last reply
                      1
                      • S Offline
                        S Offline
                        SGaist
                        Lifetime Qt Champion
                        wrote on 29 Dec 2015, 22:58 last edited by
                        #14

                        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

                        Interested in AI ? www.idiap.ch
                        Please read the Qt Code of Conduct - https://forum.qt.io/topic/113070/qt-code-of-conduct

                        K 1 Reply Last reply 29 Dec 2015, 23:10
                        1
                        • S SGaist
                          29 Dec 2015, 22:58

                          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

                          K Offline
                          K Offline
                          kshegunov
                          Moderators
                          wrote on 29 Dec 2015, 23:10 last edited by
                          #15

                          @SGaist
                          Ok. Thank you!

                          Read and abide by the Qt Code of Conduct

                          1 Reply Last reply
                          1

                          13/15

                          29 Dec 2015, 22:54

                          • Login

                          • Login or register to search.
                          13 out of 15
                          • First post
                            13/15
                            Last post
                          0
                          • Categories
                          • Recent
                          • Tags
                          • Popular
                          • Users
                          • Groups
                          • Search
                          • Get Qt Extensions
                          • Unsolved