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 a command to recompute dependencies state #28

Closed
wants to merge 9 commits into from
Closed

Add a command to recompute dependencies state #28

wants to merge 9 commits into from

Conversation

saimn
Copy link
Contributor

@saimn saimn commented Feb 25, 2015

Ref #25

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 216c69c on saimn:reset-dep into * on pydoit:master*.


class ResetDep(DoitCmdBase):
name = "reset-dep"
doc_purpose = "recompute the state of file dependencies"
Copy link
Member

Choose a reason for hiding this comment

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

what do you think of: "recompute and save the state of file dependencies without executing actions"

@schettino72
Copy link
Member

thanks for the patch. I just did a quick review. I will take a closer look on weekend...

please add :

  • an entry on CHANGES file
  • docs

dep_manager=pdepfile)
cmd_list._execute()
got = output.getvalue()
assert "processed t2\n" == got
Copy link
Member

Choose a reason for hiding this comment

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

Apart from checking the output "saying" the new value was saved, can it also check that the new value was actually saved ?

@schettino72
Copy link
Member

Please updated the files bash_completion_doit and zsh_completion_doit. These files are generated using doit tabcompletion

@saimn
Copy link
Contributor Author

saimn commented Mar 24, 2015

Hi @schettino72,
Sorry for the delay before getting back to this PR, I was busy with other things. I have updated the code to take care of all your comments (or at least I hope so :)).

@@ -502,6 +501,12 @@ def get_value(self, task_id, key_name):
raise Exception(msg % (task_id, key_name))
return values[key_name]

def get_results(self, task_name):
"""get all saved results from a task
Copy link
Member

Choose a reason for hiding this comment

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

Each task has only 1 result, so it doesnt make sense to say "get all". And the function should be called get_result().

@schettino72
Copy link
Member

Thanks for your work. No worries about taking a long time :)

Please also update(merge) your branch with master (but dont squash your commits)

@coveralls
Copy link

Coverage Status

Coverage increased (+0.0%) to 99.76% when pulling 2f97b1e on saimn:reset-dep into 1f9c3c7 on pydoit:master.

@@ -106,7 +106,10 @@ def print_usage(cmds):
print('')
print("Commands")
for cmd in sorted(six.itervalues(cmds), key=attrgetter('name')):
six.print_(" doit %s \t\t %s" % (cmd.name, cmd.doc_purpose))
# the command name is padded with spaces and must be 16 character
Copy link
Member

Choose a reason for hiding this comment

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

it wont look that good but it is ok if the command has more that 16 chars.

@schettino72
Copy link
Member

merged. github doesnt detected the merge because I squashed your commits...

thanks for the high quality patches!
I did some minor changes. please check.

@schettino72 schettino72 added this to the 0.28 milestone Mar 27, 2015
@saimn
Copy link
Contributor Author

saimn commented Mar 27, 2015

Looks good, thanks for the thorough review ! :)

@schettino72
Copy link
Member

@saimn sometimes I feel bad for being so picky on reviews hehehe

By the way, if you wish, you are welcome to get more involved with the project reviewing my commits and others. And I am looking forward for more contributions :)

@saimn
Copy link
Contributor Author

saimn commented Mar 27, 2015

@saimn sometimes I feel bad for being so picky on reviews hehehe

It is not a bad thing :D, I tend to do the same on my projects, and doit code base is quite complex with a lot of different use cases, so it's hard for a new contributor to understand every piece of code.
I still have several things I would like to contribute, so I will try to get more involved if time permits.

@saimn saimn deleted the reset-dep branch March 27, 2015 08:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants