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

Command line options -f and -k may be positioned before command. Fixes #204. #258

Closed
wants to merge 4 commits into from
Closed

Command line options -f and -k may be positioned before command. Fixes #204. #258

wants to merge 4 commits into from

Conversation

barpod
Copy link

@barpod barpod commented Jun 7, 2018

This PR fixes #204.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.05%) to 99.692% when pulling 30f6fb2 on barpod:position-of-command-wide-cmd-line-options into 088d2ba on pydoit:master.

@barpod barpod changed the title Command line options -f and -k may be positioned before command Command line options -f and -k may be positioned before command. Fixes #204. Jun 8, 2018
@barpod
Copy link
Author

barpod commented Jun 8, 2018

AppVeyor build fails because of

@pytest.fixture
I can reproduce this on my local machine. Is it a common issue?

@schettino72
Copy link
Member

@barpod just a clarification as you are claiming this was "fixed" before by you.

First your solution breaks very basic use cases like.

$ doit -k list task_name
$ doit -f my_dodo.py task_name

Apart from that your implementation has very poor code quality with copy and paste of defined options.

My solution works not only for hard-coded parameters but for any option parameter that comes that is defined by the task loader.

Next time if you want to contribute it is recommended that you discuss what an accepted solution would be.

@barpod
Copy link
Author

barpod commented Jun 18, 2018

@schettino72
I can agree, that code in the PR has poor quality and it breaks basic use cases. But looking at your approach to discussion (instead on discussing the PR you chose to fix the issue yourself and claim the bounty) I think that asking you about an accepted solution would result in the same situation: you fixing the issue. Anyway, I retracted my claim, so I hope you are satisified now.

@barpod barpod deleted the position-of-command-wide-cmd-line-options branch June 18, 2018 11:31
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.

Inconsistent expected position of command wide command line options (-k, -f)
3 participants