Feature/inplace #36
Feature/inplace #36
Conversation
AHReccese
requested review from
sadrasabouri and
sepandhaghighi
and removed request for
sepandhaghighi
|
@sadrasabouri reminder! |
CHANGELOG.md
Outdated
| @@ -10,6 +10,9 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. | |||
| - `pptx` and `xlsx` support | |||
| - `get_microsoft_format` function in `util.py` | |||
| - `SECURITY.md` | |||
| - `inplace` flag in clear, clear_all, update and update_all | |||
There was a problem hiding this comment.
- I think it's better to use
parameterfor function instead offlag - It's better to separate them into three lines
| @@ -10,6 +10,9 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. | |||
| - `pptx` and `xlsx` support | |||
| - `get_microsoft_format` function in `util.py` | |||
| - `SECURITY.md` | |||
| - `inplace` flag in clear, clear_all, update and update_all | |||
| - `inplace` flag in CLI | |||
| - `inplace` tests | |||
There was a problem hiding this comment.
I think we never reported the addition of tests to change log.
There was a problem hiding this comment.
I'm always not sure what to add or what not to add, I wish we could have a solid formula for this.
I can remove it but in PyMilo we mention per category test case additions, you can check it out in pymilo's CHANGELOG.md
README.md
Outdated
| @@ -62,30 +62,33 @@ DMeta is an open source Python package that removes metadata of Microsoft Office | |||
|
|
|||
| ## Usage | |||
| ### In Python | |||
| #### Clear metadata for a .docx file | |||
| ⚠️ You can use the `in_place` flag in functions to indicate if the action should affect the original file(s). | |||
There was a problem hiding this comment.
Use `in_place` to apply the changes directly to the original file.
README.md
Outdated
| @@ -96,24 +99,26 @@ update_all(CONFIG_FILE_PATH) | |||
|
|
|||
| ### CLI | |||
| ⚠️ You can use `dmeta` or `python -m dmeta` to run this program | |||
| ⚠️ You can use the `inplace` flag in CLI to indicate if the action should affect the original file(s). | |||
There was a problem hiding this comment.
Use `in_place` to apply the changes directly to the original file.
| #### Version | ||
| ```console | ||
| dmeta -v | ||
| dmeta --version | ||
| ``` | ||
| ### Clear metadata for a .docx file | ||
| #### Clear metadata for a .docx file in place |
There was a problem hiding this comment.
So there should be default mode usage of it (with in_place = False) in read me, too.
There was a problem hiding this comment.
I'm sorry, I'm not sure if I understood your point, could you please re-explain it?
There was a problem hiding this comment.
my bad.
I meant, right now all the examples in README (all the above Python scripts) are using in_place=True. It's good to have a Python example for in_place=False (default mode) too.
There was a problem hiding this comment.
They are not I guess, in each function, if you do not specify in_place it will fallbacks to False as the default option.
so the clear_all example and the update_all example work in in_place=False mode.
Good?
dmeta/__main__.py
Outdated
| '--inplace', | ||
| action="store_true", | ||
| default=False, | ||
| help="the `inplace` command specifies whether the requested action should be taken in place." |
There was a problem hiding this comment.
the `in_place` flag applies the changes directly to the original file.
dmeta/functions.py
Outdated
| """ | ||
| Clear all the editable metadata in the given microsoft file. | ||
|
|
||
| :param microsoft_file_name: name of microsoft file | ||
| :type microsoft_file_name: str | ||
| :param in_place: a flag specifies whether clearance should be taken in place |
dmeta/functions.py
Outdated
| """ | ||
| Clear all the editable metadata in any microsoft file in the current directory. | ||
|
|
||
|
|
||
| :param in_place: a flag specifies whether clearances should be taken in place |
dmeta/functions.py
Outdated
| """ | ||
| Update all the editable metadata in the given microsoft file according to the given config file. | ||
|
|
||
| :param config_file_name: name of .json config file | ||
| :type config_file_name: str | ||
| :param microsoft_file_name: name of microsoft file | ||
| :type microsoft_file_name: str | ||
| :param in_place: a flag specifies whether update should be taken in place |
dmeta/functions.py
Outdated
| """ | ||
| Update all the editable metadata in any microsoft file in the current directory according to the given config file. | ||
|
|
||
| :param config_file_name: name of .json config file | ||
| :type config_file_name: str | ||
| :param in_place: a flag specifies whether update should be taken in place |
|
Dear Sadra, Thank you so much for your review. |
Please read my comment above, and if you have any comment to address it better, let me know. |
Reference Issues/PRs
#21
What does this implement/fix? Explain your changes.
Any other comments?