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

Add the --ignore-undeclared imports option #158

Merged
merged 7 commits into from
Feb 17, 2023

Conversation

Nour-Mws
Copy link
Collaborator

@Nour-Mws Nour-Mws commented Feb 16, 2023

This is the symmetrical change to #151. The name --ignore-undeclared-imports symmetrical to --ignore-unused-deps would be too long. Let me know if you'd prefer renaming the old one or making the current one longer to keep the symmetry.

As with #151, the change will only impact one action (--check-undeclared):

  • --list-imports: imports are still parsed as before, the ignored list does not extend to parsed imports.
  • --check-unused: only undeclared imports may be ignored. If a dependency is unused, including it in the ignored list will have no effect. It will still be reported as unused.
  • If an import is both used and has been declared in dependencies, including it in the ignore list will not make it unused.

Note: Since there is some refactoring in the last commits, I suggest reviewing commit by commit.

- Imports are still parsed as before, the ignored list does not extend
to parsed imports.
- Only undeclared imports may be ignored. If a dependency is unused,
including it in the ignored list will have no effect.
- If an import is both used and has been declared in dependencies,
including it in the ignore list will not make it unused.
fawltydeps/utils.py Outdated Show resolved Hide resolved
fawltydeps/check.py Outdated Show resolved Hide resolved
tests/test_cmdline.py Outdated Show resolved Hide resolved
@Nour-Mws Nour-Mws force-pushed the nour/add-ignore-undeclared-imports branch from 6f7524d to b7c2c54 Compare February 17, 2023 06:39
@Nour-Mws Nour-Mws added this to the First public release milestone Feb 17, 2023
@Nour-Mws
Copy link
Collaborator Author

I updated the help message of the new option, see 5420333.

Copy link
Collaborator

@mknorps mknorps left a comment

Choose a reason for hiding this comment

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

This looks very good. The beauty of symmetry <3
Also works like a charm. The comments in --help are very helpful, and they provide a way to add a single name and a list to the --ignore options! 🚀

One further action point discussed in different places and maybe deserving its own issue:

  • It would be great that for compare_imports_to_depenencies function we had 3 arguments:
    • gathered imports
    • gathered dependencies
    • additional options
      It is related also to the configuration options in pyproject.toml.

@jherland
Copy link
Member

One further action point discussed in different places and maybe deserving its own issue:

  • It would be great that for compare_imports_to_depenencies function we had 3 arguments:

    • gathered imports
    • gathered dependencies
    • additional options
      It is related also to the configuration options in pyproject.toml.

Yes, in my ongoing settings work, that is exactly what I'm aiming for: a Settings object that collect all our configuration from command-line and pyproject.toml, and is then passed around to whoever needs it. Might even end up in the JSON...

@Nour-Mws Nour-Mws merged commit 5b9f806 into main Feb 17, 2023
@Nour-Mws Nour-Mws deleted the nour/add-ignore-undeclared-imports branch February 17, 2023 09:03
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

Successfully merging this pull request may close these issues.

3 participants