Skip to content
This repository was archived by the owner on Oct 30, 2018. It is now read-only.

[CORE-1509] Merge parse2syncano with syncano-cli#9

Merged
opalczynski merged 11 commits intodevelopfrom
CORE-1509
May 23, 2016
Merged

[CORE-1509] Merge parse2syncano with syncano-cli#9
opalczynski merged 11 commits intodevelopfrom
CORE-1509

Conversation

@opalczynski
Copy link
Copy Markdown
Contributor

No description provided.

@opalczynski
Copy link
Copy Markdown
Contributor Author

@forgems pls take a look :)

Comment thread README.rst Outdated

::

syncanocli login
Copy link
Copy Markdown
Contributor

@forgems forgems May 20, 2016

Choose a reason for hiding this comment

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

Why have you renamed command to syncanocli instead of just syncano ?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

You think just syncano is enough? I caaaan revert it - no problem :)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

As far as I remember from sprint planning, we were talking about 'syncano' name :)

Comment thread syncano_cli/parse_to_syncano/config.py Outdated
CONFIG_VARIABLES_NAMES = ['PARSE_MASTER_KEY', 'PARSE_APPLICATION_ID', 'SYNCANO_APIROOT',
'SYNCANO_ADMIN_API_KEY', 'SYNCANO_INSTANCE_NAME']

PARSE_APPLICATION_ID = os.getenv('P2S_PARSE_APPLICATION_ID', '')
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Remove P2S_ prefixes from these environement variables.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

K.

@opalczynski
Copy link
Copy Markdown
Contributor Author

@forgems Could pls take a look in new commands management?

@forgems
Copy link
Copy Markdown
Contributor

forgems commented May 23, 2016

IMHO it's overkill, and time to move to something that is already used by other projects, has good documentation and is tested. It will also lower entry barrier for new developers. Something like http://docopt.org/ (docker-compose uses it) or http://click.pocoo.org/5/. We shouldn't focus on command line parsing but on defining functionalities. I did this with 2 functions because I knew argparse before, and I didn't need any fancy functionality. IMHO use of metaclasses is reserved for libraries and frameworks, not CLI tools ;)

@opalczynski
Copy link
Copy Markdown
Contributor Author

@forgems gonna merge this :) And think in next sprint about change the argparse with another tool. Also will redesign one more time new command process adding.

@opalczynski opalczynski merged commit 3ed2a1f into develop May 23, 2016
@opalczynski opalczynski deleted the CORE-1509 branch May 23, 2016 12:25
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants