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

Removing "rule generic-catchall-00800" #83

Closed
wants to merge 1 commit into from

Conversation

Maarc
Copy link
Contributor

@Maarc Maarc commented Oct 1, 2015

It is generating only false positives and its content is already fully covered by our very nicht EJB/JPA/Server resources reports.

…tives, that are already catched in our nice custom reports
@jsight
Copy link
Member

jsight commented Oct 1, 2015

@mareknovotny - Can you take a look at this one?

@mareknovotny
Copy link
Contributor

@jsight sure.

@Maarc
Copy link
Contributor Author

Maarc commented Oct 4, 2015

@mareknovotny

As described in the pull request, this rule is generating an important amount of false positives.

Considered that we already have dedicated EJB / JPA ... reports describing what the application is doing, there is no point at keeping this rule. We do not need it any more. It is only generating "noise".

@mareknovotny
Copy link
Contributor

@Maarc I got your point and on that problem I noticed that my original change is missing the important thing to add hints/classifications only if there is not already hint added - this should be in all catchall rules to show up only if there is not any specifics about the matched group.

This change should resolve your concerns with false positive, please try it from #87.

@Maarc
Copy link
Contributor Author

Maarc commented Oct 5, 2015

Hi Marek,

#87 does not solve my "problem". I just want to remove this one rule because it is already covers by other reports (dedicated EJB / JPA reports) and only produces noise.

I still think, that we should comment out this rule.

best regards,

Marc

@jsight
Copy link
Member

jsight commented Oct 5, 2015

@rdruss - I agree with @Maarc. What do you think? And is there anything we need to do to document or get approval for this change?

@mareknovotny
Copy link
Contributor

@Maarc @jsight @rdruss I did that in mareknovotny@cb3ea8c

@jsight
Copy link
Member

jsight commented Oct 5, 2015

@mareknovotny incorporated this into #87 and I have merged that one.

@jsight jsight closed this Oct 5, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants