Details

    • Similar Issues:
    • Released As:
      gerrit-code-review-0.2

      Description

      Hi,

      It would be super great to be able to submit a file based (given file + lineno) comments and not only change comments.

      This will enable to make use of gerrit as frontend for managing warnings/errors of build, static code analysis (such as sonar or style) etc...

      As far as I imagine there can be two ways to perform this:

      1. The simple - have an optional array of [

      {file, lineno, comment}

      ] provided to the current API.

      2. The cool - use a transaction syntax such as:

      withGerritReview() {
          gerritAddComment("file1", 12, "bla bla")
          gerritAddComment("file2", 44, "bla bla sdf")
          gerritSetFlag("Code-Review", "+1")
      }
      

      Please also keep in mind parallel work. I could not understand how the magic of triggering is working due to lack of documentation, however, if a parallel job is invoked it should share the context/state of the parent job so that the parallel job can also add its information, two options here as well:

      1. The simple - parallel job has its own comments, share is only the change information, can be given as standard job parameters.

      2. The super-cool - the parallel job, if called within transaction context will be able to append to the existing transaction, share the change information + the state of transaction.

      Thank you for working on this, I am looking forward to be able to switch.
      Alon

        Attachments

          Activity

          Hide
          lucamilanesio Luca Domenico Milanesio added a comment -

          That's a very cool idea: let me work on it and finalise for presenting at the next forthcoming Jenkins World Summit in Nice

          Show
          lucamilanesio Luca Domenico Milanesio added a comment - That's a very cool idea: let me work on it and finalise for presenting at the next forthcoming Jenkins World Summit in Nice
          Hide
          lucamilanesio Luca Domenico Milanesio added a comment -

          The way I am going to implement this will reflect the way you typically work with Gerrit:

          • Add comments "as-you-go" and leave them in draft
          • When review action is triggered on the change (the "Click Reply" button) then all the draft comments are published

          If you add comments in a set of parallel jobs controlled by the main one, then all the comments generated will be published together when the main job completes or fails.

          Show
          lucamilanesio Luca Domenico Milanesio added a comment - The way I am going to implement this will reflect the way you typically work with Gerrit: Add comments "as-you-go" and leave them in draft When review action is triggered on the change (the "Click Reply" button) then all the draft comments are published If you add comments in a set of parallel jobs controlled by the main one, then all the comments generated will be published together when the main job completes or fails.
          Show
          lucamilanesio Luca Domenico Milanesio added a comment - https://github.com/jenkinsci/gerrit-code-review-plugin/pull/17 Code is under review
          Hide
          alonbl Alon Bar-Lev added a comment -

          That's great!

           

          I've read the documentation, and it looks nice, the use of draft and publish one time is good idea.

           

          The problem is that from documentation I do not understand how to install the plugin and if the current gerrit trigger plugin is required or not to trigger the job, I would like to remove the current plugin if possible. I also do not understand what component will eventually submit the draft comments.

           

          As there is no closure, I am unsure how a job which was not directly triggered can comment, for example, I have CI which runs only integration tests, it is not triggered directly from the review, but is aware of the review and like to put comments.

           

          I guess all the above are installation related and not per this specific feature.

           

          Thanks!

           

          Show
          alonbl Alon Bar-Lev added a comment - That's great!   I've read the documentation, and it looks nice, the use of draft and publish one time is good idea.   The problem is that from documentation I do not understand how to install the plugin and if the current gerrit trigger plugin is required or not to trigger the job, I would like to remove the current plugin if possible. I also do not understand what component will eventually submit the draft comments.   As there is no closure, I am unsure how a job which was not directly triggered can comment, for example, I have CI which runs only integration tests, it is not triggered directly from the review, but is aware of the review and like to put comments.   I guess all the above are installation related and not per this specific feature.   Thanks!  
          Hide
          alonbl Alon Bar-Lev added a comment -

          One minor comment I wanted to add to github but it seems to be dead.

          Maybe I do not understand the logic...

          The default label cannot be verified as it will impact the previous settings, for example:

          gerritComment label: CI-Static, score:1, message: 'Static analysis was passed'
          gerritComment path:'path/to/file', line: 10, message: 'invalid syntax' 

          Or:

          gerritComment score:1, message: 'ok'
          gerritComment path:'path/to/file', line: 10, message: 'invalid syntax' 

          Or even:

          gerritComment score:1, message: 'ok'
          gerritComment message: 'I have more to say'

          In all cases the verified label will be modified by the last gerritComment invocation which is not expected.

          Options:

          1. Have null as default label, this allows differentiate between intentional and non intentional.
          2. Split the gerritComment into two commands, one to control the commit and one to control the file based comments, however even in this situation adding plain messages should be handled somehow, so back to (1).

          Thanks!

           

          Show
          alonbl Alon Bar-Lev added a comment - One minor comment I wanted to add to github but it seems to be dead. Maybe I do not understand the logic... The default label cannot be verified as it will impact the previous settings, for example: gerritComment label: CI-Static, score:1, message: 'Static analysis was passed' gerritComment path: 'path/to/file' , line: 10, message: 'invalid syntax' Or: gerritComment score:1, message: 'ok' gerritComment path: 'path/to/file' , line: 10, message: 'invalid syntax' Or even: gerritComment score:1, message: 'ok' gerritComment message: 'I have more to say' In all cases the verified label will be modified by the last gerritComment invocation which is not expected. Options: Have null as default label, this allows differentiate between intentional and non intentional. Split the gerritComment into two commands, one to control the commit and one to control the file based comments, however even in this situation adding plain messages should be handled somehow, so back to (1). Thanks!  

            People

            • Assignee:
              lucamilanesio Luca Domenico Milanesio
              Reporter:
              alonbl Alon Bar-Lev
            • Votes:
              1 Vote for this issue
              Watchers:
              4 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: