comments in PR

[OneCyrus]

any chance we get commenting of webpack errors/warnings inside pull requests as comments. especially with typescript and es/tslint errors in webpack this would be awesome.

sonarqube is doing it and it's a great experience: https://blogs.msdn.microsoft.com/devops/2016/06/02/sonarqube-code-analysis-issues-integration-into-pull-requests/

[jkanczler]

Interesting idea. I'm thinking on it. Maybe I can do this using the VSTS REST API to create comments from the build.

[jkanczler]

Thanks for the suggestion. It won't be that hard to implement as I've checked it. The vso-node-api library will make my life easier to implement this. Though I have to find out the details when to create a new comment thread and when to close a comment thread (as errors/warnings be appeared and be fixed).

E.g. after every build I have to close existing comment threads expect those that still remains an active error/warning. But if a file has multiple errors let's say and one of the errors is fixed. The position of the remaining errors are changed in the file, because of the fix. What should happen? Close those comments? Reopen a new thread for the new position?

[OneCyrus]

thanks for looking into this.

good questions! i'm trying to think about potential matching strategies. how likely is it that the same error happens on another line (same error message, just different line number)? is there a way to move a thread? but i guess closing old line and opening a new thread would be good start. showing the thread on a wrong line is probably confusing.

[jkanczler]

Found an answer for this. Fortunately, MS moves the comments appropriately if the file changes. VSTS is tracking pull request iterations (a push to the pull request) and you can get back the current position of the comments correctly.

For example (where the last two parameters are the identifier of the iteration and baseIteration):

    gitApi.getThreads(repositoryId, pullRequestId, project, iteration, baseIteration)

It means, hopefully, I can pair the actual errors and warnings to the current line that I can get back from the API. If the line number and the content of the comment is a match, then it's the same error, I can leave the comment in an active state.

[jkanczler]

I have another challenges. Loaders... Every loader has a different format for errors and warnings.

[OneCyrus]

oh that‘s probably a challenge. too bad. my main use-case would be typescript with ts-loader. Is it mainly parsing the filename and line number?

[jkanczler]

My plan is to support the popular loaders then. E.g.:

• ts-loader,
• tslint-loader,
• babel-loader,
• eslint-loader,
• sass-loader,
• less-loader,
• css-loader.

Those are coming up into my mind right now.

[OneCyrus]

sounds good. that are the loaders we are using too. nothing to add right now.

[jkanczler]

Still, lots of outstanding questions are there when can I close already existing comments. Until solving those the feature is still not available. :-(

[OneCyrus]

what questions are not answered? may be I can help you find ways to solve them.

[jkanczler]

Unfortunately testing the solution is not that straightforward. I have to release a working version to test the enhancement. As you could see there were some pull request merge already. But during the testing I've found serious issues, like how to track warnings. It's not clear to me yet I need to work on the "algorithm" that can properly merge/close/create new pull request comments. So I rolled back the feature until I'll have time to implement this feature in a stable way. Hope it makes sense. But I haven't forgot the idea, hopefully I'll have time for this to implement soon.