-
Notifications
You must be signed in to change notification settings - Fork 3
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
Improved help text to fix issue #35, updated setup.py to fix issue #39 #43
Conversation
octopose/deploy.py
Outdated
parser.add_argument('--wait', action="store_true", | ||
help="Wait and poll for deploy. Octopose will continually poll the Octopus Deploy Tasks till they are complete") | ||
parser.add_argument('--force', action='store_true', | ||
help='Force deployment. Ensures the package is re-downloaded even if it is already deployed into the target environment') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tiny one, can this say "Ensures the package is re-deployed even if the selected version is already deployed into the target environment"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you split into 2 seperate PRs one for each issue.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM - just a few nits / queries.
Thanks for contributing would be good to see more step up 🤘
Feel free to keep both issues in here but would be good to split further issues.
parser.add_argument('--wait', action="store_true", help="Wait and poll for deploy") | ||
parser.add_argument('--force', action='store_true', help='Force deployment') | ||
parser.add_argument('--wait', action="store_true", | ||
help="Wait and poll for deploy. Octopose will continually poll the Octopus Deploy Tasks till they are complete") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
complete.
parser.add_argument('--wait', action="store_true", | ||
help="Wait and poll for deploy. Octopose will continually poll the Octopus Deploy Tasks till they are complete") | ||
parser.add_argument('--force', action='store_true', | ||
help='Force deployment. Ensures the package is re-deployed even if the selected version is already deployed into the target environment') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
environment.
or remove from -e
👍
parser.add_argument('--wait', action="store_true", | ||
help="Wait and poll for deploy. Octopose will continually poll the Octopus Deploy Tasks till they are complete") | ||
parser.add_argument('--force', action='store_true', | ||
help='Force deployment. Ensures the package is re-deployed even if the selected version is already deployed into the target environment') | ||
parser.add_argument('-v', '--verbose', action='store_true', help="Do not suppress logging from deploy scripts.") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would update this message ... verbose says we are getting extra debug info IMO ... up to you guys. But would change to something along the lines of Verbose logging gives extra contextual output.
Also, what is the default on this arg? I get confused with this one ... by default we should not log at verbose level.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Default is do not log at the verbose level
No description provided.