Skip to content

Conversation

@bbrands02
Copy link
Contributor

No description provided.

@github-actions
Copy link
Contributor

👋 @bbrands02
Thank you for raising your pull request.
Please make sure you have followed our contributing guidelines. We will review it as soon as possible. In the meanwhile make sure your PR checks the following boxes

  • Is based on an issue
  • Has been locally tested
  • Has been tested with the admin UI
  • Has been discussed with the development team in an open channel

}

$responseStatusCode = 400;
$message = 'ome database connections failed: '.(string) $mongoDBFailCount.' out of '.(string) $mongoDBCount.' connections';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ome?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In case he combines 2 messages he uses lower s and if this is the start of the message he uses capital S

$status = 'warn';
}

$responseStatusCode = 400;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would throw a 500-range here

$status = 'fail';
$errors = null;
$outputMessage = $outputMessage ? $outputMessage .= ' and could not fetch error logs: '.$e->getMessage() : $outputMessage = 'Could not fetch error logs: '.$e->getMessage();
$responseStatusCode = 400;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here, 400 range sounds like the test does something wrong, instead of that there's an error on the server

*
* @param MetricsService $metricsService The metrics service
* @param ParameterBagInterface $parameters The Parameter bag
* @param EntityManagerInterface $entityManager The Parameter bag
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

docblock description incorrect

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants