On 02/22/2016 11:09 AM, Filip Skola wrote:


----- Original Message -----
On 02/10/2016 09:17 AM, Milan Kubík wrote:
On 02/09/2016 04:19 PM, Milan Kubík wrote:
On 01/28/2016 10:42 AM, Filip Skola wrote:
----- Original Message -----
On 01/25/2016 11:11 AM, Filip Skola wrote:
----- Original Message -----
On 01/15/2016 03:38 PM, Filip Skola wrote:
Hi,

sending rebased patch.

F.

----- Original Message -----
Hello,

sorry for delays. The patch no longer applies to master. Rebase
it,
please.

Milan

----- Original Message -----
From: "Filip Škola" <fsk...@redhat.com>
To: "Milan Kubík" <mku...@redhat.com>
Cc: freeipa-devel@redhat.com
Sent: Wednesday, 9 December, 2015 7:01:02 PM
Subject: Re: [Freeipa-devel] [PATCH 0002] Refactor
test_group_plugin

On Mon, 7 Dec 2015 17:49:18 +0100
Milan Kubík <mku...@redhat.com> wrote:

On 12/03/2015 08:15 PM, Filip Škola wrote:
On Mon, 30 Nov 2015 17:18:30 +0100
Milan Kubík <mku...@redhat.com> wrote:

On 11/23/2015 04:42 PM, Filip Škola wrote:
Sending updated patch.

F.

On Mon, 23 Nov 2015 14:59:34 +0100
Filip Škola <fsk...@redhat.com> wrote:

Found couple of issues (broke some dependencies).

NACK

F.

On Fri, 20 Nov 2015 13:56:36 +0100
Filip Škola <fsk...@redhat.com> wrote:

Another one.

F.
Hi, the tests look good. Few remarks, though.

1. Please, use the shortes copyright notice in new modules.

          #
          # Copyright (C) 2015  FreeIPA Contributors see
COPYING for
license #

2. The tests `test_group_remove_group_from_protected_group` and
`test_group_full_set_of_objectclass_not_available_post_detach`
were not ported. Please, include them in the patch.

Also, for less hassle, please rebase your patches on top of
freeipa-mkubik-0025-3-Separated-Tracker-implementations-into-standalone-pa.patch

Which changes the location of tracker implementations and
prevents
circular imports.

Thanks.

Hi,

these cases are there, in corresponding classes. They are marked
with the original comments. (However I can move them to separate
class if desirable.)

The copyright notice is changed. Also included a few changes
in the
test with user without private group.

Filip
NACK

linter:
************* Module tracker.group_plugin
ipatests/test_xmlrpc/tracker/group_plugin.py:257:
[E0102(function-redefined), GroupTracker.check_remove_member]
method
already defined line 253)

Probably a leftover after the rebase made on top of my patch.
Please
fix it. You can check youch changes by make-lint script before
sending them.

Thanks

Hi,

I learned to use make-lint!

Thanks,
F.

Hello,

NACK, pylint doesn't seem to like the way the fixtures are imported
(pytest does a lot of runtime magic) [1].
One possible solution would be [2]. Though, I don't think this
would be
a good idea in our environment. I suggest to create the fixtures
on per
module basis.


[1]: http://fpaste.org/311949/53118942/
[2]:
https://pytest.org/latest/fixture.html#using-fixtures-from-classes-modules-or-projects


--
Milan Kubik


Hi,

the fixtures were copied into corresponding module. Please note
that this
patch has a dependence on my patch 0001 (user plugin).

Filip
Linter:
************* Module ipatests.test_xmlrpc.tracker.group_plugin
W:100,26: Calling a dict.iter*() method (dict-iter-method)

please use dict.items

--
Milan Kubik


Hi, sorry. This has been fixed in this patch.

Filip
ACK, thanks for the patience. :)

Sorry, there are some other things I need clarified. NACK.
Mail will follow later.

What is the purpose of `make_fixture_detach` in your patches? They are
not used anywhere and the finalizer does nothing.

--
Milan Kubik


Hi,

none, I guess, probably a leftover copied from the tracker in the early days. 
Deleting the function.

Filip
Ack.

--
Milan Kubik

--
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