Remove unused entity.manager service from Drupal\system\Form\ModulesListForm

This is a tiny change. In reviewing this I:

• Looked over the parent issue and read this issue entirely.
• Checked for inheritors of the class being changed. There are none.
• Verified there is a test. There is: ModulesListFormWebTest. The test is not comprehensive. But the entityManager was doing absolutely nothing in this class so I am not concerned.
• Checked that all the dead code is removed: it is.
• Checked for Drupal coding standards - there is a nitpick-level problem.

+++ b/core/modules/system/src/Form/ModulesListForm.php
@@ -144,12 +133,11 @@ public static function create(ContainerInterface $container) {
-  public function __construct(ModuleHandlerInterface $module_handler, ModuleInstallerInterface $module_installer, KeyValueStoreExpirableInterface $key_value_expirable, AccessManagerInterface $access_manager, EntityManagerInterface $entity_manager, AccountInterface $current_user,  RouteMatchInterface $route_match, TitleResolverInterface $title_resolver, RouteProviderInterface $route_provider, MenuLinkManagerInterface $menu_link_manager) {
+  public function __construct(ModuleHandlerInterface $module_handler, ModuleInstallerInterface $module_installer, KeyValueStoreExpirableInterface $key_value_expirable, AccessManagerInterface $access_manager, AccountInterface $current_user,  RouteMatchInterface $route_match, TitleResolverInterface $title_resolver, RouteProviderInterface $route_provider, MenuLinkManagerInterface $menu_link_manager) {

There are two spaces after a comma in the function signature.

I remove the two spaces separation and upload a patch, the interdiff and a screenshot of phpunit result

patch re-tested and passed this time.

assigning correct status

@jmolivas ModulesListFormWebTest is a Simple Test so phpunit does not execute it.

In addition to what I did in the review in #7, I manually tested the form and found it is working fine. Based on the code standards fix, this is RTBC.

Thanks all for your work here!

The beta evaluation here isn't entirely complete. It isn't listed as either a prioritized change or unfrozen, which would mean that, by default, it should be postponed. Going through the flowchart for allowed changes during the beta:

1. It's not a feature request.
2. It's not unfrozen (it changes more than CSS, markup, tests, strings, migrate).
3. It's not critical.
4. It's not a prioritized change (the goal is not to improve usability, accessibility, security, performance, etc. nor fix a bug).
5. It's not major.
6. There's no mention on the issue of it reducing fragility (which needs committer signoff). In this case? Yeah, it is. Removing dead, unused, untested code is one of the things that we discussed as a potential reduction to fragility. So I'm in favor of prioritizing this during the beta.

So once someone confirms that it's a prioritized change (in this case, with a committer's signoff), then we evaluate the impact versus the disruption. That section of the beta evaluation could also use some work because we need to consider more than just core. The disruption would be that any extending class (core/contrib/custom code) would now have a broken constructor. So we should ask ourselves, how likely is it that there are alternate module list form implementations out there? It seems like something there could be a usecase to override in contrib and distros. It seems less likely that they exist already. I think that updating the constructor is a small disruption for those modules, so the impact of removing unneeded code outweighs this disruption.

I've updated the beta evaluation to reflect these things. Please do similarly in other issues, especially normal and minor tasks. :) Consider whether cleanups should be postponed.

Didn't actually need to set the status back, since I took the time to do the beta evaluation myself. In general, though, non-docs normal tasks without the beta evaluation (or with incomplete ones) can be "Needs work". :)

And so, per #14, this issue is a prioritized change as per https://www.drupal.org/core/beta-changes and its benefits outweigh any disruption. Committed and pushed to 8.0.x. Thanks!

I decided a constructor change for a form builder does not need a change record.

@xjm Thank you for your careful attention.