Users can comment on pipeline run history.#699
Conversation
a81b183 to
321bcd1
Compare
There was a problem hiding this comment.
LocalizedMessage.cannotViewPipeline(pipelineName) isn't correct. You need to create & use LocalizedMessage.cannotOperatePipeline(pipelineName). You will need to also add message PIPELINE_CANNOT_OPERATE in localize.xml & localize_ja.xml.
|
can you please add the |
There was a problem hiding this comment.
pipelineLabel is unique but they might contain dots i.e. . and this might not work well when you do '#comment-form-14.3.0'. You can replace the dots with somethingelse for work around or use something totally different (pipelineCounter).
variable name should be pipelineCounter if we are planning to use pipeline-counter here.
bug: i tried this patch with pipeline label set to this.is.${COUNT} in config and the pop-up does not open onclick of button.
|
Sorry i have mentioned same issue multiple times. din't want to miss it next time around. Thanks for the PR. nice feature & well implemented :) |
|
Done as part of #707
|
|
Thanks for all the comments @srinivasupadhya! We'll integrate all the changes you mentioned. Also, thanks for taking the time to point out the changes in all the places they needed to be made (even if they are the same change). |
|
Thanks @gajwani. Can this go in 14.3.1? We are thinking of a release end of Nov with 4 features:
Also, we talked to our UX @nandhakumarr and he was of the opinion that the experience should be like this. Your thoughts? |
|
@srinivasupadhya, sorry to hijack the conversation, but where is this roadmap defined? I can't see anything like this in the roadmap page on go.cd? |
|
@matt-richardson, actually term "plugin api changes" is a bit misleading because existing plugin api's are not getting changed in any way and therefore all existing plugins would continue to work as is. However what we working on is to come up with an alternate set of plugin api's (message based). Currently only delete artifact extension point which is under development makes use of the new api's .The main motivation behind this change has been to avoid situations wherein changes to the plugin api causes existing plugins to break (something that has happened in past). Eventually every other plugin should move to these new api's to leverage advantages offered by it but don't worry till that happens, the older plugin api would continue to be around and existing plugins would continue to work as before. Regarding roadmap, thanks for bringing to our notice that this information is missing from there. Since work started on message based api's as part of development of artifact extension point (part of roadmap), it got missed being explicitly mentioned in the roadmap. But point taken, it very much deserves place of its own there. We would be updating the roadmap soon and also shortly posting details of the new plugin api on the mailing lists for opinions / suggestions from the community. Thanks once again! |
Signed-off-by: Matthew Boedicker <mboedicker@pivotal.io>
Signed-off-by: Matthew Boedicker <mboedicker@pivotal.io>
Signed-off-by: Matthew Boedicker <mboedicker@pivotal.io>
Signed-off-by: Matthew Boedicker <mboedicker@pivotal.io>
Signed-off-by: Matthew Boedicker <mboedicker@pivotal.io>
Signed-off-by: Matthew Boedicker <mboedicker@pivotal.io>
Signed-off-by: Matthew Boedicker <mboedicker@pivotal.io>
Signed-off-by: Matthew Boedicker <mboedicker@pivotal.io>
Signed-off-by: Frank Kotsianas <fkotsianas@pivotal.io>
Signed-off-by: Frank Kotsianas <fkotsianas@pivotal.io>
Signed-off-by: Frank Kotsianas <fkotsianas@pivotal.io>
Signed-off-by: Frank Kotsianas <fkotsianas@pivotal.io>
Signed-off-by: Matthew Boedicker <mboedicker@pivotal.io>
Update pipeline history service integration test to test error message. Signed-off-by: Matthew Boedicker <mboedicker@pivotal.io>
Signed-off-by: Matthew Boedicker <mboedicker@pivotal.io>
Signed-off-by: Matthew Boedicker <mboedicker@pivotal.io>
Signed-off-by: Matthew Boedicker <mboedicker@pivotal.io>
321bcd1 to
0e84bbe
Compare
|
Thanks for the detailed review. We made all the changes suggested. As far as sanitizing the input before writing to the database, we felt it was safer to sanitize on the way out because of different output formats. For example, when the comment is in the API JSON it should be escaped as JSON and not HTML. We are still looking into the UI changes suggested by @nandhakumarr. |
|
Hmm..Ok. Other things look good to me so accepting this. We had a discussion about the UX/UI. We were thinking of doing minimal changes to make it look a little better. If we cannot do that we might hide this feature (may be with a flag that people can use to turn it on). Will keep you posted on this. Please update the documentation. May be here. Thanks a lot for the feature! :) |
Users can comment on pipeline run history.
|
Update: We could not find time to put that little polish on this. @arvindsv has implemented feature toggle support in Go. We have turned this feature off with the help of that & you will be able to turn it on if you wish to. |
|
In case you missed this tweet :) |
Each pipeline run can have a single comment written next to it. This can be used to share failure information among teams looking at the pipelines without needing to review the logs.
Viewing comments, note the different button text:
Editing a comment:
