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
Forum Updated to NodeBB v4.3 + New Features

Pushing code for review

Scheduled Pinned Locked Moved Solved Qt Contribution
gerritgitguiwidgetsqpa
15 Posts 2 Posters 7.6k Views 2 Watching
  • 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.
  • SGaistS Offline
    SGaistS Offline
    SGaist
    Lifetime Qt Champion
    wrote on 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

    kshegunovK 1 Reply Last reply
    2
    • SGaistS SGaist

      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.

      kshegunovK Offline
      kshegunovK Offline
      kshegunov
      Moderators
      wrote on last edited by
      #5

      @SGaist
      Ok, thank you!

      Read and abide by the Qt Code of Conduct

      kshegunovK 1 Reply Last reply
      1
      • kshegunovK kshegunov

        @SGaist
        Ok, thank you!

        kshegunovK Offline
        kshegunovK Offline
        kshegunov
        Moderators
        wrote on 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
        • SGaistS Offline
          SGaistS Offline
          SGaist
          Lifetime Qt Champion
          wrote on 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

          kshegunovK 1 Reply Last reply
          2
          • SGaistS SGaist

            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

            kshegunovK Offline
            kshegunovK Offline
            kshegunov
            Moderators
            wrote on 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
            • SGaistS Offline
              SGaistS Offline
              SGaist
              Lifetime Qt Champion
              wrote on 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

              kshegunovK 2 Replies Last reply
              3
              • SGaistS SGaist

                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.

                kshegunovK Offline
                kshegunovK Offline
                kshegunov
                Moderators
                wrote on 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
                • SGaistS SGaist

                  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.

                  kshegunovK Offline
                  kshegunovK Offline
                  kshegunov
                  Moderators
                  wrote on 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
                  • SGaistS Offline
                    SGaistS Offline
                    SGaist
                    Lifetime Qt Champion
                    wrote on 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

                    kshegunovK 1 Reply Last reply
                    1
                    • SGaistS SGaist

                      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)

                      kshegunovK Offline
                      kshegunovK Offline
                      kshegunov
                      Moderators
                      wrote on 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
                      • SGaistS Offline
                        SGaistS Offline
                        SGaist
                        Lifetime Qt Champion
                        wrote on 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

                        kshegunovK 1 Reply Last reply
                        1
                        • SGaistS SGaist

                          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

                          kshegunovK Offline
                          kshegunovK Offline
                          kshegunov
                          Moderators
                          wrote on last edited by
                          #15

                          @SGaist
                          Ok. Thank you!

                          Read and abide by the Qt Code of Conduct

                          1 Reply Last reply
                          1

                          • Login

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