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

Fix #868: Option to ignore globally-namespaced classes from MissingImport #869

Merged
merged 7 commits into from
Mar 10, 2021

Conversation

danepowell
Copy link
Contributor

Type: feature
Issue: Resolves #868
Breaking change: no

See #868 for motivation. Adds a property ignore-global to the MissingImport rule that will cause it to ignore classes in the global namespace (i.e. classes that do not contain a backslash in their FQN).

This still needs tests but I'd like maintainer approval and guidance before proceeding with that, since I'm a little intimidated by the unit test architecture. I don't know how to incorporate an optional flag into the test cases.

As an aside, kudos on maintaining a very well-organized and documented codebase, it was pretty easy to grok.

@tvbeek
Copy link
Member

tvbeek commented Feb 11, 2021

@danepowell thanks for the PR. Maybe you already got an notification / email from the GitHub actions but there are failing test. If I take a quick look it is the result of not having a default value. Can you check check and update it? (Not having a default value that keeps the current logic should also make it a breaking change)

@danepowell
Copy link
Contributor Author

Thanks, I fixed the broken test and also added a test case for the new option.

Co-authored-by: Kyle <kylekatarnls@users.noreply.github.com>
Copy link
Member

@tvbeek tvbeek left a comment

Choose a reason for hiding this comment

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

I have a small request for change, just to make it more explicit. After that I think it is perfect.

src/site/rst/rules/cleancode.rst Outdated Show resolved Hide resolved
Co-authored-by: Tobias van Beek <github@tjvb.nl>
@tvbeek tvbeek merged commit 484d625 into phpmd:master Mar 10, 2021
@tvbeek
Copy link
Member

tvbeek commented Mar 10, 2021

Thanks for the PR

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

Successfully merging this pull request may close these issues.

Exclude global namespace classes from MissingImport rule
3 participants