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

Sonardotnt 382 #8

Closed
wants to merge 6 commits into
base: master
from

Conversation

Projects
None yet
2 participants
@peterstevens130561
Copy link

peterstevens130561 commented Jan 2, 2014

Only changes related to SONARDOTNT-382.
Added unite tests

@dbolkensteyn

This comment has been minimized.

Copy link
Member

dbolkensteyn commented Jan 31, 2014

Indeed CSharpTypeVisitor was horribly implemented, and did not support nested namespaces.

I have just refactored it to be more readable, hence I no longer can apply your patch as is.

But I'll manually go through the changes you did and apply them.

Thank you!

@dbolkensteyn

This comment has been minimized.

Copy link

dbolkensteyn commented on c1a3669 Jan 31, 2014

Regarding m2e, I recommend not to change the poms, and use this plugin instead: https://github.com/SonarSource/sonar-settings/tree/master/eclipse/m2e-error-disabler-plugin

@dbolkensteyn

This comment has been minimized.

Copy link

dbolkensteyn commented on d90481c Jan 31, 2014

Ah didn't see this one ;)

@dbolkensteyn

This comment has been minimized.

Copy link
Member

dbolkensteyn commented Jan 31, 2014

It's definitely better to add the test on nested namespaces in the existing CSharpTypeVisitorTest.java file rather than create a new file just for that.

@dbolkensteyn

This comment has been minimized.

Copy link
Member

dbolkensteyn commented Jan 31, 2014

It is important that test class names end with "Test" to be certain that they will be picked up by surefire - referring to "CSharpResourcesBridgeTestNested"

@peterstevens130561

This comment has been minimized.

Copy link

peterstevens130561 commented Jan 31, 2014

You're welcome

Peter

Op 31 januari 2014 om 12:03 schreef Dinesh Bolkensteyn
notifications@github.com:

Indeed CSharpTypeVisitor was horribly implemented, and did not support nested
namespaces.

I have just refactored it to be more readable, hence I no longer can apply
your patch as is.

But I'll manually go through the changes you did and apply them.

Thank you!


Reply to this email directly or view it on GitHub
#8 (comment)
.

@dbolkensteyn

This comment has been minimized.

Copy link
Member

dbolkensteyn commented Jan 31, 2014

Thanks, I've applied this.

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