I cannot apply your patch on master branch, missing blobs, can you rebase please?

On 06.01.2016 11:47, Oleg Fayans wrote:
Any chance this patch can be merged this week?

On 12/14/2015 02:08 PM, Oleg Fayans wrote:
Hi Martin,

On 12/11/2015 05:58 PM, Martin Basti wrote:

On 11.12.2015 17:28, Oleg Fayans wrote:
+    myre = re.compile(".*Backed up to (?P<backup>.*?)\n.*")
IMO this regexp is not good.

1)
please name it better than "myre"
Done

2)
initial '.*' is not needed because regexp does not start with '^' and
you use search() later

3)

trailing '.*' is not needed as well, because it does not end with '$'

4)
You can use re.MULTILINE that will parse string per lines

path_re = re.compile("^Backed up to (?P<backup>.*)$", re.MULTILINE)
Used it, thanks!

5)
+    matched = myre.search(result.stdout_text + result.stderr_text)
Why do you need search in both stderr and stdout?
Because of this bug: https://fedorahosted.org/freeipa/ticket/5484

Martin^2





--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Reply via email to