Skip to content
This repository has been archived by the owner on Dec 4, 2020. It is now read-only.

Fix for using multiple flags with adop compose init #166

Merged
merged 1 commit into from
Feb 15, 2017

Conversation

statlus
Copy link
Contributor

@statlus statlus commented Nov 22, 2016

Wrapped the case statement at the start of adop compose init() with a while loop so all options are read instead of just the first one.

cmd/compose Outdated
shift
;;
*)
shift
Copy link
Contributor

@nickdgriffin nickdgriffin Nov 23, 2016

Choose a reason for hiding this comment

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

Was this added to prevent an infinite loop from occurring? (Line 104)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added this to just shift through any invalid flags, maybe I should echo an error message as well?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah I think that'd be useful - I think it should print a helpful message and exit please.

@statlus statlus force-pushed the feature/composefix branch 2 times, most recently from 9e7d798 to daa4e82 Compare December 6, 2016 15:27
@statlus
Copy link
Contributor Author

statlus commented Dec 6, 2016

I've changed the catchall option to echo a message and exit 1.

@nickdgriffin
Copy link
Contributor

Finally got around to testing this, and it's all dandy from Ubuntu 16.04. I'll give it a shot from Windows too soon.

@anton-kasperovich Would you be able to give it a quick go at some point on OSX please?

@nickdgriffin
Copy link
Contributor

Tested from Windows 7 with the latest version of Docker Toolbox and it works fine.

@statlus Could you rebase on master just so the history is tidier please?

@anton-kasperovich
Copy link
Contributor

Tested from macOS Sierra (10.12.3) and it works. Thank you!

@statlus
Copy link
Contributor Author

statlus commented Feb 15, 2017

Hi @nickdgriffin, I've rebased the change on master.

@nickdgriffin
Copy link
Contributor

Great, thanks.

So, LGTM!

@nickdgriffin nickdgriffin merged commit acd56db into Accenture:master Feb 15, 2017
@statlus statlus deleted the feature/composefix branch February 16, 2017 09:48
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.

3 participants