Add name_in_archive option to erl_tar:add/3 #105
Conversation
There are 2 erl_tar:add(). One has an option as an extra argument. It is better to allow this option to be part of the normal options. The extra function is kept for backwards compatibility.
|
Patch has passed first testings and has been assigned to be reviewed |
| @@ -77,7 +77,8 @@ close(_) -> | |||
| %% Adds a file to a tape archive. | |||
|
|
|||
| add(File, Name, Options) -> | |||
| add(File, Name, Name, Options). | |||
| NameInArchive = proplists:get_value( name_in_archive, Options, Name ), | |||
There was a problem hiding this comment.
Code style violation: There should be no extra spaces after a '(' and before a ')'.
There was a problem hiding this comment.
Sorry about the style violation. If the patch is found worthy I will adjust the style.
|
Can you explain why it would be better to have the name of the file file in the option list instead of an extra agurment? Since I would expect each file added to a tar file to have a unique name, having to put a new name into the option list for each call to erl_tar:add/3 does not seem any easier or more logical to me. |
|
It is (slightly) more consistent to have all options in the Options variable. To single out one option (name_in_archive) and let it be an extra argument (see erl_tar:add/4) does not agree with me. |
Try all shrinkers in sampleshrink/1
There are 2 erl_tar:add(). One has an option as an extra argument.
It is better to allow this option to be part of the normal options.
The extra function is kept for backwards compatibility.