Better separation of concerns between Team and Commissioner

[marcandre]

Excluding non-relevant cops is a duty of Team, not Commissioner.

Before this PR:

• a second call to Commissioner#investigate could be missing a cop that was excluded previously
• a force might be added by Team for a single cop that is later judged to be non relevant by a Commissioner but the later can't exclude the force now
• cops judged not relevant by the Commissioner were still looped through (e.g. in autocorrect_all_cops)

PR also includes some code cleanup. This is a prelude to #7968

[marcandre]

Argh, this is all good but the spec_helper needs to be fixed. It also shows it was duplicating the finding the forces. It should create a Team, not a Commissionner, but then this shows that Team doesn't have the right interface: it should accept cops, not cop classes. On hold.

[marcandre]

I refactored Team.new to accept an array of cops instead of an array of cop classes. Team doesn't actually need to create the cops itself and is more useful to accept cops. Previous functionality was moved to Team.mobilize.

I feel that's the cleanest API, but it would be easy for Team.new to accept an array of cops or an array of cop classes, thus maintaining compatibility. Is there any code out there that might call Team.new and should we care, especially for a v1? /cc @bbatsov

[bbatsov]

I feel that's the cleanest API, but it would be easy for Team.new to accept an array of cops or an array of cop classes, thus maintaining compatibility.

Compatibility with what? :-) I think this class is not used outside the main gem. I did a quick grep in some extensions and I don't see any usages.

[bbatsov]

Btw, this might also be a good opportunity to actually document the responsibilities of the class - I see currently they are "FIXME". :D

[marcandre]

Cool. I'll add a short doc and a Changelog entry

[marcandre]

Compatibility with what? :-)

Now we know that the answer is, at least, pronto-rubocop and erb_lint 😅.

[bbatsov]

So we do. :-)