Skip to content

Ignore PHP namespace statements #328

New issue

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

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

Already on GitHub? # to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

tillkruss
Copy link

@tillkruss tillkruss commented Jul 20, 2018

We're seeing hundreds of code duplication warnings for namespace statements.

screen shot 2018-07-20 at 9 08 17 am

#245 didn't include the actual namespace statements and this PR covers the case when there is a space between the namespace and use blocks.

@tillkruss
Copy link
Author

The CircleCI tests are don't seem to be working: https://circleci.com/gh/codeclimate/codeclimate-duplication

Any tips @larkinscott?

@tillkruss tillkruss changed the title [WIP] Ignore PHP namespace statements Ignore PHP namespace statements Jul 20, 2018
@tillkruss
Copy link
Author

Any feedback on this PR @toddmazierski @brynary?

@larkinscott
Copy link
Contributor

👋 @tillkruss Thanks for the PR! This seems reasonable to me.

Looking into this CI failure now.

@larkinscott
Copy link
Contributor

@tillkruss Thanks for bringing that CI failure to our attention. This should be fixed now if you rebase on top of master.

@tillkruss tillkruss reopened this Jul 20, 2018
@tillkruss
Copy link
Author

@larkinscott I rebased the branch, but now other tests are failing:

rspec ./spec/cc/engine/analyzers/php/main_spec.rb:138 # CC::Engine::Analyzers::Php::Main#run can parse php 7 code
rspec ./spec/cc/engine/analyzers/php/main_spec.rb:93 # CC::Engine::Analyzers::Php::Main#run runs against complex files

Both fail due to:

 expected: > 0
            got:   0

I assume the issue previously was the namespace and my PR eliminates the issue?

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants