cli: pull command migrated to docopts #145

Merged
merged 1 commit into from Dec 3, 2015

Conversation

Projects
None yet
2 participants
Collaborator

sergiusens commented Dec 3, 2015

Signed-off-by: Sergio Schvezov sergio.schvezov@canonical.com

snapcraft/lifecycle.py
import logging
import os
import sys
import shutil
+
+# Non-Standard Library modules
snapcraft/lifecycle.py
import yaml
+# Snapcraft modules
snapcraft/commands/pull.py
+
+"""
+
+# Non-Standard Library modules
snapcraft/commands/pull.py
+# Non-Standard Library modules
+from docopt import docopt
+
+# Snapcraft modules
snapcraft/lifecycle.py
+
+ for part in parts:
+ common.env = config.build_env_for_part(part)
+ part.pull()
@elopio

elopio Dec 3, 2015

Member

I'm confused here. It's doing only pull because you haven't migrated the other commands yet?

@sergiusens

sergiusens Dec 3, 2015

Collaborator

yes and pull is always going to be executed anyways; an

if cmds == 'pull':
    part.pull()

will always evaluate to True when I loop over the commands as it is the first one in the list.

Also, yeah, the other commands don't exist 😉

@sergiusens

sergiusens Dec 3, 2015

Collaborator

I added a comment

snapcraft/tests/test_commands_pull.py
+ self.assertEqual(len(state), 1, 'Expected only one line in the state '
+ 'file for the pull1 part')
+ self.assertEqual(state[0], 'pull', 'Expected the state file for pull1 '
+ 'to be \'pull\'')
@elopio

elopio Dec 3, 2015

Member

why don't you use "pull" instead of escaping the quotes?

@sergiusens

sergiusens Dec 3, 2015

Collaborator

For message consistency, but I can change it.

snapcraft/tests/test_commands_pull.py
+
+ for i in [0, 2]:
+ self.assertFalse(os.path.exists(parts[i]['part_dir']),
+ 'Expected for only to be a part dir for pull1')
@elopio

elopio Dec 3, 2015

Member

This is hard to read. I'm not sure what are you trying to say.
Maybe something like: "Pulled wrong part."

@sergiusens

sergiusens Dec 3, 2015

Collaborator

Sure.

Collaborator

sergiusens commented Dec 3, 2015

Updated with all your comments @elopio
Thanks for the review.

Member

elopio commented Dec 3, 2015

👍 , pending the s/stage/step

cli: pull command migrated to docopts
Signed-off-by: Sergio Schvezov <sergio.schvezov@canonical.com>

sergiusens added a commit that referenced this pull request Dec 3, 2015

Merge pull request #145 from sergiusens/06-docopts-pull
cli: pull command migrated to docopts

@sergiusens sergiusens merged commit dbd9b09 into snapcore:new-cli Dec 3, 2015

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage increased (+2.04%) to 80.299%
Details

@sergiusens sergiusens deleted the sergiusens:06-docopts-pull branch Dec 3, 2015

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