Cleanup: simplify flag parsing and merge into mflag (rebase ++) by SvenDowideit · Pull Request #8158 · moby/moby

SvenDowideit · 2014-09-22T07:17:45Z

this is to get #7551 updated for merge.

I've added the missing non-global validation functions, and rebased

SvenDowideit · 2014-09-22T07:42:01Z

LK4D4 · 2014-09-22T07:43:02Z

gofmt failed
Also probably you should close #7551

SvenDowideit · 2014-09-22T11:01:54Z

(I haven't closed the original because i'm not 100% sure the somewhat large number of changes I've had to make are right)

looks like I'm going to need some help - there's an integration test failure that I don't know enough about:

[PASSED]: create - echo test123
SIGQUIT: quit
PC=0x42f353

goroutine 17 [syscall]:
runtime.notetsleepg(0x7ff3ee9e2f60, 0xdf8475800)
    /usr/local/go/src/pkg/runtime/lock_futex.c:198 +0x46 fp=0x7ff3ee9e2f38 sp=0x7ff3ee9e2f08
runtime.MHeap_Scavenger()
    /usr/local/go/src/pkg/runtime/mheap.c:532 +0xa3 fp=0x7ff3ee9e2fa8 sp=0x7ff3ee9e2f38
runtime.goexit()
    /usr/local/go/src/pkg/runtime/proc.c:1445 fp=0x7ff3ee9e2fb0 sp=0x7ff3ee9e2fa8
created by runtime.main
    /usr/local/go/src/pkg/runtime/proc.c:207

goroutine 16 [chan receive, 4 minutes]:
testing.RunTests(0x8a9088, 0x9fc160, 0x107, 0x107, 0x77b101)
    /usr/local/go/src/pkg/testing/testing.go:505 +0x923
testing.Main(0x8a9088, 0x9fc160, 0x107, 0x107, 0xa03140, 0x0, 0x0, 0xa03140, 0x0, 0x0)
    /usr/local/go/src/pkg/testing/testing.go:435 +0x84
main.main()
    github.com/docker/docker/integration-cli/_test/_testmain.go:621 +0x10c

goroutine 19 [finalizer wait, 1 minutes]:
runtime.park(0x414860, 0x9fdb70, 0x9fa529)
    /usr/local/go/src/pkg/runtime/proc.c:1369 +0x89
runtime.parkunlock(0x9fdb70, 0x9fa529)
    /usr/local/go/src/pkg/runtime/proc.c:1385 +0x3b
runfinq()
    /usr/local/go/src/pkg/runtime/mgc0.c:2644 +0xcf
runtime.goexit()
    /usr/local/go/src/pkg/runtime/proc.c:1445

goroutine 20 [syscall, 9 minutes]:
os/signal.loop()
    /usr/local/go/src/pkg/os/signal/signal_unix.go:21 +0x1e
created by os/signal.init·1

SvenDowideit · 2014-09-22T11:31:00Z

mmm, and then I ran the tests again, and they passed....

tiborvass · 2014-09-22T16:31:12Z

This should be removed. It was moved to api/client/commands.go

@SvenDowideit Arggg, forgot to remove this one. Sorry about that! Can you amend?

tiborvass · 2014-09-22T16:33:45Z

@SvenDowideit I'll send you a PR, it will be easier than commenting.

SvenDowideit · 2014-09-22T23:28:02Z

@tiborvass awesome, thanks :)

tiborvass · 2014-09-23T15:16:16Z

LGTM

LK4D4 · 2014-09-23T16:43:33Z

Could you do exact check of error? We have already one false-pass test where we don't check error.

I vote for a simple: strings.Contains(out, "Conflicting options")

Also, the Test has to start with TestRun.

jessfraz · 2014-09-23T17:03:01Z

LGTM

tiborvass · 2014-09-23T20:02:01Z

@SvenDowideit I forgot to remove all this block which is redundant since we already set those fields when creating the config variable.

Signed-off-by: Solomon Hykes <[email protected]> Docker-DCO-1.1-Signed-off-by: Solomon Hykes <[email protected]> (github: SvenDowideit)

Docker-DCO-1.1-Signed-off-by: Sven Dowideit <[email protected]> (github: SvenDowideit)

Signed-off-by: Tibor Vass <[email protected]> Docker-DCO-1.1-Signed-off-by: Tibor Vass <[email protected]> (github: SvenDowideit)

SvenDowideit · 2014-09-24T02:39:12Z

updating, building and testing. - docker build -t tag . and --filter work. so its looking up.

Docker-DCO-1.1-Signed-off-by: Sven Dowideit <[email protected]> (github: SvenDowideit)

LK4D4 · 2014-09-24T07:08:44Z

These files are not properly gofmt'd:
 - integration-cli/docker_cli_run_test.go

tiborvass · 2014-09-24T23:09:03Z

Ping @SvenDowideit

SvenDowideit changed the title ~~Cleanup flags sven~~ Cleanup: simplify flag parsing and merge into mflag (rebase ++) Sep 22, 2014

SvenDowideit mentioned this pull request Sep 22, 2014

Cleanup: simplify flag parsing and merge into mflag #7551

Closed

SvenDowideit force-pushed the cleanup-flags-sven branch from faa36d4 to 0e7e99e Compare September 22, 2014 10:58

tiborvass reviewed Sep 22, 2014
View reviewed changes

SvenDowideit force-pushed the cleanup-flags-sven branch from 43bd28c to 968ab8e Compare September 23, 2014 12:21

LK4D4 reviewed Sep 23, 2014
View reviewed changes

tiborvass reviewed Sep 23, 2014
View reviewed changes

Solomon Hykes and others added 6 commits September 24, 2014 12:29

Refactor flag parsing utilities in opts/

8957f0e

Signed-off-by: Solomon Hykes <[email protected]> Docker-DCO-1.1-Signed-off-by: Solomon Hykes <[email protected]> (github: SvenDowideit)

Merge opts/ into mflag/ to simplify the interface

3c3ceda

Signed-off-by: Solomon Hykes <[email protected]> Docker-DCO-1.1-Signed-off-by: Solomon Hykes <[email protected]> (github: SvenDowideit)

Fix regression in pkg/mflag.IP.String

4e7ae0a

Signed-off-by: Solomon Hykes <[email protected]> Docker-DCO-1.1-Signed-off-by: Solomon Hykes <[email protected]> (github: SvenDowideit)

add non-global Var's for the extracted Opt types

6ce3842

Docker-DCO-1.1-Signed-off-by: Sven Dowideit <[email protected]> (github: SvenDowideit)

merges due to rebase

61ed3ba

Docker-DCO-1.1-Signed-off-by: Sven Dowideit <[email protected]> (github: SvenDowideit)

Fix docker create

11d6b5f

Signed-off-by: Tibor Vass <[email protected]> Docker-DCO-1.1-Signed-off-by: Tibor Vass <[email protected]> (github: SvenDowideit)

address test feedback

406f419

Docker-DCO-1.1-Signed-off-by: Sven Dowideit <[email protected]> (github: SvenDowideit)

SvenDowideit force-pushed the cleanup-flags-sven branch from 968ab8e to 406f419 Compare September 24, 2014 03:41

Conversation

SvenDowideit commented Sep 22, 2014

Uh oh!

SvenDowideit commented Sep 22, 2014

Uh oh!

LK4D4 commented Sep 22, 2014

Uh oh!

SvenDowideit commented Sep 22, 2014

Uh oh!

SvenDowideit commented Sep 22, 2014

Uh oh!

tiborvass Sep 22, 2014

Choose a reason for hiding this comment

Uh oh!

tiborvass Sep 23, 2014

Choose a reason for hiding this comment

Uh oh!

tiborvass commented Sep 22, 2014

Uh oh!

SvenDowideit commented Sep 22, 2014

Uh oh!

tiborvass commented Sep 23, 2014

Uh oh!

LK4D4 Sep 23, 2014

Choose a reason for hiding this comment

Uh oh!

tiborvass Sep 23, 2014

Choose a reason for hiding this comment

Uh oh!

jessfraz commented Sep 23, 2014

Uh oh!

tiborvass Sep 23, 2014

Choose a reason for hiding this comment

Uh oh!

SvenDowideit commented Sep 24, 2014

Uh oh!

LK4D4 commented Sep 24, 2014

Uh oh!

tiborvass commented Sep 24, 2014

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants