Gitlab and consolidate_comments

[tleish]

Does the consolidate_comments config option work for Gitlab? I have the following in .pronto.yml and it doesn't seem to be consolidating them.

all:
  exclude:
    - 'spec/**/*'
gitlab:
  slug: XXX
  api_private_token: XXX
  api_endpoint: XXX
  gitlab_api_httparty_options: "{verify: false}"
  format: "**%{runner} [%{level}]:** %{msg}"
max_warnings: 150
verbose: false
consolidate_comments: true

Or perhaps I'm mis-understanding the feature. The problem is, when pronto runs and finds multiple items to report, it posts multiple comments in Gitlab and ultimately several people get spammed with lots of emails instead of just one comment and one email. I was hoping this would solve it.

[ivanovaleksey]

@tleish this feature joins multiple comments for single line of code. For instance, given simple code with Rubocop offences

def foo_method
  a = 1
  b =1
end

$ pronto run -c origin/develop --staged -f text
app/foo.rb:2 W: Useless assignment to variable - `a`.
app/foo.rb:3 W: Useless assignment to variable - `b`.
app/foo.rb:3 W: Surrounding space missing for operator `=`.

As you can see there are 2 comments for line 3.

And if you run Gitlab formatter you will receive only 2 messages

$ pronto run -c origin/develop --staged -f gitlab
[#<struct Pronto::Comment sha="9e1c883f6762ad2a39c080b42e2cf03cff8c9c50", body="Useless assignment to variable - `a`.", path="app/foo.rb", position=2>, #<struct Pronto::Comment sha="9e1c883f6762ad2a39c080b42e2cf03cff8c9c50", body="- Useless assignment to variable - `b`.\n- Surrounding space missing for operator `=`.", path="app/foo.rb", position=3>]

Note that second message's body is joined comments. Without consolidate_comments: true in config file you would receive 3 separate comments.

[ivanovaleksey]

By the way consolidate looks like too official (and long) word IMO. Wouldn't it be better to use something more explicit - join - or something more git-like - squash? /cc @mmozuras

[mmozuras]

@tleish could you expand on what kind of behavior you expect? A single comment under a pull request with all problems?

Even if consolidate_comments is not what you're looking for, we could consider introducing a separate feature/runner 🙂.

@doomspork would love your thoughts on @ivanovaleksey comment about consolidate vs. join/squash 🙂

[tleish]

We use Gitlab. The complaint I'm getting my team is when they push a single commit, they get multiple emails (an email per issue reported by a pronto library, since they get an email each time a comment is left). If this do a single push, and then only get one email for that single push it would be better. Even, if they could get a single summarized email for single commit it would be better. Perhaps that single summarized comment to have hyperlinks which jump you to that line of code.

This might cause more confusion though if a summarized comment is left identifying 3 issues... then they fix 1 of the issues and push, but that original summarized comment identifying 3 issues still exists and would be visible during a merge request. So, I'm not sure this would work.

[ivanovaleksey]

I use Pronto with GitLab. But I don't use email as notification service. Instead I have Slack integration. In this case receiving multiple comments is totally fine IMO. Do you have an option to use your team messenger instead of email?

[tleish]

@ivanovaleksey - we use Slack also, but have not integrated it with Gitlab. In your setup, does gitlab send the notifications to your personal slack, or to a shared slack channel?

[doomspork]

There are only two hard things in Computer Science: cache invalidation and naming things. -- Phil Karlton

@mmozuras I would personally be hesitant to rename a configuration option that's been released for the better part of a year "just because". If there's a influx of issues/bugs/complaints because it's confusing then I think it's warranted. Other than that it seems pedantic.

No one is typing consolidated_comments so often that it's taking away from productivity 😁

@tleish / @mmozuras maybe we could have a new "report" feature that gives you a single summary of all of your issues?

We had a similar issue with my previous employer with there being too many notifications but that to me is indicative of a bigger problem. When we had individuals producing dozens upons dozens of messages then there was a discussion around code quality, using git hooks, and running things locally.

[ivanovaleksey]

@tleish out of the box GitLab integration with Slack sends notifications to specified channel. But that is fine in my case since everyone could see code offences. I argree with @doomspork that things should be run locally too.

[tleish]

I agree for something like Brakeman for strong security purposes. But for more opinionated static styling analysis, it can be considered more opinionated and "suggestions" rather than "failures". For failures, I'm okay users receiving multiple emails for each offense. However, for "suggestion" type analysis an alternative would be nice.

[mmozuras]

@mmozuras I would personally be hesitant to rename a configuration option that's been released for the better part of a year "just because".

👍

@tleish / @mmozuras maybe we could have a new "report" feature that gives you a single summary of all of your issues?

Sure. I think a new formatter with this behavior would be fine 🙂. What do you think?

[doomspork]

@mmozuras if there's enough interest in a "report" I'd be happy to help nail down the features/requirements and take a stab at building it. @tleish would that work for you?

Going back over this I'm wondering if we couldn't solve this by whitelist/blacklist-ing certain "events"

[mmozuras]

@doomspork that would be great 👍

Going back over this I'm wondering if we couldn't solve this by whitelist/blacklist-ing certain "events"

Could you expand on what you mean?

[doomspork]

@mmozuras disregard for now, it was a half baked thought 😀

[tleish]

@doomspork - I'm interested in collaborating a solution.

[doomspork]

Sounds great @tleish! Do you want me to get a branch started that we can work with? How would you like to proceed? 😁