Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Display InsightClass also when Insight has no detail #168

Conversation

Jibbarth
Copy link
Collaborator

@Jibbarth Jibbarth commented Jun 8, 2019

Q A
Bug fix? yes
New feature? no
Fixed tickets #...

When an Insight doesn't implement HasDetail, we cannot see it's Class with -v option.

// example
• [Architecture] Property per class limit: (ObjectCalisthenics\Sniffs\Metrics\PropertyPerClassLimitSniff)
  src/Domain/Collector.php:13: "class" has too many properties: 40. Can be up to 10 properties.

• [Architecture] The name property in the `composer.json` contains the default value

This fix allow that

• [Architecture] Property per class limit: (ObjectCalisthenics\Sniffs\Metrics\PropertyPerClassLimitSniff)
  src/Domain/Collector.php:13: "class" has too many properties: 40. Can be up to 10 properties.

• [Architecture] The name property in the `composer.json` contains the default value: (NunoMaduro\PhpInsights\Domain\Insights\ComposerMustContainName)

@Jibbarth Jibbarth added the bug Something isn't working label Jun 8, 2019
Copy link
Collaborator

@olivernybroe olivernybroe left a comment

Choose a reason for hiding this comment

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

Ah yeah, this makes sense. Great work!

@nunomaduro
Copy link
Owner

@Jibbarth @olivernybroe I trust you guys to merge this kind of things. 👍

If two of the maintainers agree on the pull request, the pull request can be merged. Unless it's a huge modification, in that case it's better if we all agree on it.

@Jibbarth Jibbarth merged commit afe7b32 into nunomaduro:master Jun 8, 2019
@Jibbarth Jibbarth deleted the fix/display-insight-class-when-no-details branch June 8, 2019 15:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants