Skip to content

Add support for Star Wars characters - #2

Open
jarcec wants to merge 1 commit into
dimaspivak:masterfrom
jarcec:master
Open

Add support for Star Wars characters#2
jarcec wants to merge 1 commit into
dimaspivak:masterfrom
jarcec:master

Conversation

@jarcec

@jarcec jarcec commented Sep 9, 2017

Copy link
Copy Markdown

No description provided.

@dimaspivak dimaspivak left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Awesome contribution.

Comment thread namemysprint.py
args = parser.parse_args()

if not args.name_pack in NAME_PACKS:
print(f'Unknown name pack "{args.name_pack}", available packs: {", ".join(NAME_PACKS.keys())}')

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Use .format notation to maintain Python 3.4+ support

Comment thread namemysprint.py
def main():
parser = argparse.ArgumentParser(description='Generate the best sprint names.')
parser.add_argument('--name-pack',
action='store',

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Store is the default so this argument can be left out

Comment thread namemysprint.py
parser = argparse.ArgumentParser(description='Generate the best sprint names.')
parser.add_argument('--name-pack',
action='store',
help='Name of the pack used for generating name.',

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

No period at end of argparse help messages

Comment thread namemysprint.py
'Yoda', ]


NAME_PACKS = {'cyclists': CYCLISTS, 'starwars': STARWARS}

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Instead of this dictionary, just capitalize the argparse argument given when pulling from the list.

Comment thread README.rst
This project will generate the name of your team's next sprint by
combining a random adjective and a random name (chosen from a list of pro
cyclists).
combining a random adjective and a random name. There is a support

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Remove "a" between "is" and "support"

Comment thread namemysprint.py

def main():
parser = argparse.ArgumentParser(description='Generate the best sprint names.')
parser.add_argument('--name-pack',

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Make this a choice argument so that namepack choices get displayed when help is invoked

Comment thread namemysprint.py
'Schleck', 'Thomas', 'Ullrich', 'Uran', 'Valverde', 'VanGarderen',
'Vaughters', 'Voigt', 'Vos', 'Wiggins', 'Zabel', 'Zabriskie']

STARWARS = ['Ackbar', 'Amidala', 'Anakin', 'Andor', 'BB-8', 'Binks', 'C-3PO',

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Please alphabetize so that future contributors can easily see if a name they may want is missing.

Comment thread README.rst
combining a random adjective and a random name (chosen from a list of pro
cyclists).
combining a random adjective and a random name. There is a support
for several name packs - for example cyclists, star wars characters, ...

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Capitalize Star Wars

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Make it say "There is currently support for several name packs (e.g. cyclists, Star Wars characters) and pull requests for new ones are encouraged."

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