Skip to content
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

mount: bind-recursive: remove boolean convenience values #4671

Merged
merged 1 commit into from
Nov 22, 2023

Conversation

thaJeztah
Copy link
Member

Commit fc6976d (#4316) introduced support for the bind-recursive option on --mount, and deprecated the bind-nonrecursive option. Unlike bind-nonrecursive boolean, the bind-recursive option accepts a string value with multiple options.

For convenience, the bind-recursive option also was made to accept boolean values (true/false, 1/0). However, as the option works as the reverse of bind-nonrecursive (bind-nonrecursive=true === bind-recursive=false), the new option won't be a "drop-in" replacement, and having more options to choose from may only be adding more complexity / cognitive overload.

This patch removes support for boolean values; if we see a need to add support for boolean values in future, it would be trivial to add back this functionality.

- A picture of a cute animal (not mandatory but encouraged)

- updates fc6976d
- updates 74bace1

Commit fc6976d introduced support for the
`bind-recursive` option on `--mount`, and deprecated the `bind-nonrecursive`
option.  Unlike `bind-nonrecursive` boolean, the `bind-recursive` option
accepts a string value with multiple options.

For convenience, the `bind-recursive` option also was made to accept boolean
values (true/false, 1/0). However, as the option works as the _reverse_ of
`bind-nonrecursive` (`bind-nonrecursive=true` === `bind-recursive=false`),
the new option won't be a "drop-in" replacement, and having more options
to choose from may only be adding more complexity / cognitive overload.

This patch removes support for boolean values; if we see a need to add
support for boolean values in future, it would be trivial to add back this
functionality.

Signed-off-by: Sebastiaan van Stijn <[email protected]>
@codecov-commenter
Copy link

codecov-commenter commented Nov 22, 2023

Codecov Report

Merging #4671 (7be05a6) into master (1401f91) will increase coverage by 0.01%.
The diff coverage is 100.00%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #4671      +/-   ##
==========================================
+ Coverage   59.72%   59.73%   +0.01%     
==========================================
  Files         287      287              
  Lines       24832    24824       -8     
==========================================
- Hits        14831    14829       -2     
+ Misses       9115     9110       -5     
+ Partials      886      885       -1     

@thaJeztah
Copy link
Member Author

@AkihiroSuda @dvdksn @jalonsogo PTAL

@thaJeztah
Copy link
Member Author

Thanks for review! Let me bring this one in

@thaJeztah thaJeztah merged commit 8046bb2 into docker:master Nov 22, 2023
76 checks passed
@thaJeztah thaJeztah deleted the bind_recusrive_no_bool branch November 22, 2023 10:49
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.

5 participants