[elbe-devel] [PATCH] Add support for 'lexists' and 'append_file' for 'Filesystem'

[Torben Hohn]

Did you intend to send me a single patch ?

On Thu, May 16, 2019 at 01:57:04PM +0200, dion at linutronix.de wrote:
> From: Olivier Dion <dion at linutronix.de>
> 
> 'lexists' acts like:
> ------------------------------------------------------------
> os.path.exist(path) or os.path.islink(path)
> ------------------------------------------------------------
> on systems supporting os.stats
> 
> 'append_file' appends content to a file and can optionally change the
> file mode.
> 
> Signed-off-by: Olivier Dion <dion at linutronix.de>

Acked-by: Torben Hohn <torben.hohn at linutronix.de>

i would have preffered both functions separate.
But this is ok.

Explanation: 

"Acked-by:" is enough, to get the code in. However,
with an Acked-by comes a comment, which explains, how you could make
the reviewer even happier. (and that would give you the Reviewed-by:,
in the next iteration of the Patchqueue.

Depending on how much time you have, you reiterate over the patch.


> ---
>  elbepack/filesystem.py | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/elbepack/filesystem.py b/elbepack/filesystem.py
> index 6ced9ddb..4c331d23 100644
> --- a/elbepack/filesystem.py
> +++ b/elbepack/filesystem.py
> @@ -78,6 +78,9 @@ class Filesystem(object):
>      def exists(self, path):
>          return os.path.exists(self.fname(path))
>  
> +    def lexists(self, path):
> +        return os.path.lexists(self.fname(path))
> +
>      def mkdir(self, path):
>          os.makedirs(self.fname(path))
>  
> @@ -154,6 +157,13 @@ class Filesystem(object):
>          f.close()
>          self.chmod(path, mode)
>  
> +    def append_file(self, path, content, **kwargs):
> +        f = self.open(path, "a")
> +        f.write(content)
> +        f.close()
> +        if "mode" in kwargs:
> +            self.chmod(path, kwargs["mode"])

why are you using kwargs, and not mode=None ?
this would yield:

if mode is not None:
	self.chmod(path, mode)

Seems more elegant to me. And also makes the function signature pretty
much self documenting..
> +
>      def read_file(self, path):
>          fp = self.open(path, "r")
>          retval = fp.read()
> -- 
> 2.21.0
> 

-- 
Torben Hohn
Linutronix GmbH | Bahnhofstrasse 3 | D-88690 Uhldingen-Mühlhofen
Phone: +49 7556 25 999 18; Fax.: +49 7556 25 999 99

Hinweise zum Datenschutz finden Sie hier (Informations on data privacy 
can be found here): https://linutronix.de/kontakt/Datenschutz.php

Linutronix GmbH | Firmensitz (Registered Office): Uhldingen-Mühlhofen | 
Registergericht (Registration Court): Amtsgericht Freiburg i.Br., HRB700 
806 | Geschäftsführer (Managing Directors): Heinz Egger, Thomas Gleixner
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://lists.linutronix.de/pipermail/elbe-devel/attachments/20190516/8326f9d9/attachment.sig>