close
Skip to content

Remove deprecated cli flags#17724

Merged
calavera merged 1 commit into
moby:masterfrom
runcom:remove-depreciated-cli-flags
Nov 17, 2015
Merged

Remove deprecated cli flags#17724
calavera merged 1 commit into
moby:masterfrom
runcom:remove-depreciated-cli-flags

Conversation

@runcom

@runcom runcom commented Nov 5, 2015

Copy link
Copy Markdown
Member

@duglin

duglin commented Nov 5, 2015

Copy link
Copy Markdown
Contributor

nice clean-up!

@runcom

runcom commented Nov 5, 2015

Copy link
Copy Markdown
Member Author

I did want to take care of https://github.com/docker/docker/blob/master/docs/misc/deprecated.md#docker-content-trust-env-passphrase-variables-name-change as well but it's weird it was deprecated in 1.9 and target for removal is 1.10

@thaJeztah

Copy link
Copy Markdown
Member

Darn! So had this on my to-do list, thought this would be an easy one for me to pick up, LOL

@runcom

runcom commented Nov 5, 2015

Copy link
Copy Markdown
Member Author

There are all the tests yet to be fixed if you want to carry this lol O:)

@thaJeztah

Copy link
Copy Markdown
Member

hehe. no, feel free to take it! 😺
don't forget to update https://github.com/docker/docker/blob/master/docs/misc/deprecated.md as well, to make clear they have been removed in release 1.10

@runcom runcom force-pushed the remove-depreciated-cli-flags branch from 577afbf to cfb1957 Compare November 5, 2015 17:24
@runcom runcom force-pushed the remove-depreciated-cli-flags branch 4 times, most recently from a85a059 to 514af47 Compare November 6, 2015 09:32
@runcom

runcom commented Nov 7, 2015

Copy link
Copy Markdown
Member Author

ping @duglin @calavera @cpuguy83

@runcom runcom force-pushed the remove-depreciated-cli-flags branch from 514af47 to f484eb9 Compare November 7, 2015 17:27
@runcom

runcom commented Nov 8, 2015

Copy link
Copy Markdown
Member Author

Windows fail seems related

@runcom

runcom commented Nov 8, 2015

Copy link
Copy Markdown
Member Author

ping @jhowardmsft I can't seem to find the issue with windows :/

@lowenna

lowenna commented Nov 8, 2015

Copy link
Copy Markdown
Member

@runcom You need to kick it off again. See my IRc log from Friday evening. You've hit a common CI problem.

@runcom

runcom commented Nov 8, 2015

Copy link
Copy Markdown
Member Author

@jhowardmsft oh :( thanks for looking!

Comment thread api/client/search.go

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.

was --no-trunc deprecated? I don't see it in the deprecated doc file.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

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.

But was it listed in the deprecation docs? If not, and we still want to delete it anyway, then let's add it.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

-notrunc was deprecated and I removed it
--no-trunc in docker search wasn't deprecated and I kept it in https://github.com/docker/docker/pull/17724/files#diff-9207f9a977bc11e99cfb49522dd26292R30

why do we want to delete even --no-trunc if it wasn't listed?

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.

Oh sorry, I misread the above line - I thought you were removing it - I missed that its down below on line 30. I was used to seeing git-diff showing just the deleted words.

@duglin

duglin commented Nov 8, 2015

Copy link
Copy Markdown
Contributor

I might be misreading stuff, but the docs talk about --networking but the option is really --net. Could you include an update to the docs as part of this PR?

@duglin

duglin commented Nov 8, 2015

Copy link
Copy Markdown
Contributor

actually, I take that back. There is a --net and --networking (I think). Odd I don't see -networking on docker run --help though.

Comment thread integration-cli/docker_cli_run_test.go Outdated

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.

I don't think -net is deprecated so I think this test is still valid, no?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

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.

I'm not following. Is --net/-n deprecated? I not then shouldn't this entire testcase be kept?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'm a little concerned from the Windows side if --net=none is deprecated? What's the replacement? There will be other plumbing required in the Windows drivers if it is deprecated (along with this test)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

-n and --networking are deprecated, nothing to do with --net

The first part of the test is testing --net so it should be kept, the other -n -- I'll remove it

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Ah, good :)

@runcom runcom force-pushed the remove-depreciated-cli-flags branch from f484eb9 to 06a8511 Compare November 9, 2015 14:37
Comment thread runconfig/parse.go Outdated

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.

forgot #privileged

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Thanks tibor <3

@tiborvass

Copy link
Copy Markdown
Contributor

needs rebase

@runcom runcom force-pushed the remove-depreciated-cli-flags branch from 06a8511 to 475e5a7 Compare November 9, 2015 19:48
@runcom

runcom commented Nov 9, 2015

Copy link
Copy Markdown
Member Author

@tiborvass updated & rebased

@stevvooe

stevvooe commented Nov 9, 2015

Copy link
Copy Markdown
Contributor

@runcom I'm just going to leave this here: http://stackoverflow.com/questions/9208091/the-difference-between-deprecated-depreciated-and-obsolete.

I'm assuming that we want to make these "obsolete", rather than "decrease their monetary value".

Please excuse my pedantry. :)

@runcom

runcom commented Nov 9, 2015

Copy link
Copy Markdown
Member Author

@stevvooe ahahah I even wrote the commit message right, excuse my typo as I'm an ESL actually :)

@LK4D4

LK4D4 commented Nov 13, 2015

Copy link
Copy Markdown
Contributor

Ok, gave up on windows.
LGTM

@thaJeztah

Copy link
Copy Markdown
Member

@runcom sorry, needs a rebase :'(

Signed-off-by: Antonio Murdaca <runcom@redhat.com>
@runcom runcom force-pushed the remove-depreciated-cli-flags branch from 475e5a7 to 7929888 Compare November 15, 2015 09:40
@calavera

Copy link
Copy Markdown
Contributor

LGTM

calavera added a commit that referenced this pull request Nov 17, 2015
@calavera calavera merged commit d507acb into moby:master Nov 17, 2015
@runcom runcom deleted the remove-depreciated-cli-flags branch November 17, 2015 16:42
@thaJeztah thaJeztah added this to the 1.10 milestone Nov 20, 2015
@thaJeztah thaJeztah changed the title Remove depreciated cli flags Remove deprecated cli flags Nov 20, 2015
yongtang added a commit to yongtang/docker that referenced this pull request May 11, 2016
The old command line options have been deprecated in 1.8.0 and
eventually removed in 1.10.0 through PR moby#17724, though the
deprecated.md still shows `Target For Removal In Release`.

This fix updates the deprecated.md and changes
`Target For Removal In Release` to `Removed In Release`.

Signed-off-by: Yong Tang <yong.tang.github@outlook.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants