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

Ignore comments #3

Open
fmpwizard opened this issue Feb 5, 2012 · 2 comments
Open

Ignore comments #3

fmpwizard opened this issue Feb 5, 2012 · 2 comments

Comments

@fmpwizard
Copy link

Hi,

If a line is commented out, it is still marked as a code smell. It would be better if that did not happen (at least when commented with // , if detecting

/**
 *
/*

seems more involved.

@arosien
Copy link
Owner

arosien commented Feb 5, 2012

Yeah regex's are inherently limited, but I don't want to have to support real parsing because I think the value of sniff is in really simple smell expressions. Tools like PMD and FindBugs can look into the bytecode.

You could add // detection into your smell definitions, or as I would argue, you should delete commented code since you're not using it anyway.

One final note: perhaps you can use an Ignore clause to not flag the commented code. However Ignores only work at the file level (right now). Maybe we can add an annotation or magic token to mark a section as "ignored"?

@fmpwizard
Copy link
Author

In this one case, the comment was to tell me that I removed a [Option].get call , but because I don't have enough tests on that area yet (they have to involve Selenium in my project), I left the comment tell me that if something goes wrong in a week, or a month, this may be a good place to look at :)
I simply added a space between the dot and get and that took care of it :)

I never tried PMD but what I like about sniff is that I already use specs2, so it is an easy addition. Of course this project becomes very useful when you have larger teams, where you most likely have a hudson instance running tests for you, where you can add findbug. But it saved me some time, instead of running some grep shell script, I just write a new rule and let the test tell me files and line numbers where the problems are :)

Thanks

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

No branches or pull requests

2 participants