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 support for multiple positional path arguments #68

Closed
wants to merge 2 commits into from

Conversation

guykisel
Copy link
Contributor

This is an attempt at resolving #61

Example usage: prospector tests/test_autodetect.py setup.py prospector/tools

@guykisel
Copy link
Contributor Author

My basic manual attempts at testing this have worked, but I suspect there are a lot of edge cases that would break this.

@guykisel
Copy link
Contributor Author

I think one of the major issues to figure out in this sort of solution is how best to communicate what tools/profiles were run on what files/folders, since right now there's just one unified summary. Individual summaries per file could end up looking pretty cluttered.

@carlio
Copy link
Contributor

carlio commented Nov 14, 2014

@guykisel Thanks for the pull request, sorry I didn't get around to looking at it earlier.

I think you hit the nail on the head with your last comment - with this approach, it's very hard to communicate what settings were used in which folder. By having a separate Prospector instance per input folder, it's equivalent to a bash command like this:

for p in path1 path2 path3
do
    prospector $p
done

The bash command has the benefit of being more explicit for where it finds config too!

There are several options:

  1. only allow multi-path for files rather than folders, which removes the ambiguity, or
  2. implicitly use only the first directory listed for config, or
  3. simply document why multi-path isn't a good plan for prospector.

Personally I think 1) is the best approach but it's a bit tricky to communicate in documentation and the --help output...

@guykisel
Copy link
Contributor Author

@carlio thanks for the comments. I agree that 1) sounds best. I'm thinking that in addition to documentation and notes in --help, Prospector could raise a warning or error if a directory path is passed in along with other paths.

I'll continue experimenting with this.

@guykisel
Copy link
Contributor Author

@carlio also, I saw your update to contrib.rst about opening PRs against develop. If you like, I can close this and reopen it as a PR to develop, or I can just wait until I have something more mature before opening a new PR.

@carlio
Copy link
Contributor

carlio commented Nov 15, 2014

@guykisel No worries, the PR against master is fine, it's not a big deal at all!

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.

2 participants