Re: [Freeipa-devel] [PATCH 0128] dnszone-remove-permission should raise NotFound if permission doesn't exist

2014-09-25 Thread Martin Kosek
On 09/24/2014 06:22 PM, Martin Basti wrote:
 On 24/09/14 17:30, Martin Kosek wrote:
 On 09/24/2014 04:55 PM, Martin Basti wrote:
 Patch attached

 This probably should go to 4.0.x, 4.1 and master
 It is obvious that this interface was designed this way. So you should
 elaborate more on the should part, list use cases where current approach 
 does
 not work, link to tickets, ...


 Sorry for that, I though I broke it during refactoring.

Not so fast, pardner :-) I checked with 3.3.3 and this *was* indeed changed
during your refactoring. My main point was that you should be clear about your
intents and reasons for the patch, that a mere should is not clear to 
everybody.

ACK to your patch though, works fine and restores the behavior - time to add
tests?. I just adjusted the commit message a little before pushing.

Pushed to:
master: 2f1f1221701160ebeb4f23078adce3af59892162
ipa-4-1: 7a99f22ee0a4c5fd1d41722fa0101fee74e0c76e
ipa-4-0: 32b6eb5110b2f851fbedb39ac0d853b46087465f

Martin

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


Re: [Freeipa-devel] [PATCH 0128] dnszone-remove-permission should raise NotFound if permission doesn't exist

2014-09-25 Thread Martin Basti

On 25/09/14 09:59, Martin Kosek wrote:

On 09/24/2014 06:22 PM, Martin Basti wrote:

On 24/09/14 17:30, Martin Kosek wrote:

On 09/24/2014 04:55 PM, Martin Basti wrote:

Patch attached

This probably should go to 4.0.x, 4.1 and master

It is obvious that this interface was designed this way. So you should
elaborate more on the should part, list use cases where current approach does
not work, link to tickets, ...



Sorry for that, I though I broke it during refactoring.

Not so fast, pardner :-) I checked with 3.3.3 and this *was* indeed changed
during your refactoring. My main point was that you should be clear about your
intents and reasons for the patch, that a mere should is not clear to 
everybody.

ACK to your patch though, works fine and restores the behavior - time to add
tests?. I just adjusted the commit message a little before pushing.

Pushed to:
master: 2f1f1221701160ebeb4f23078adce3af59892162
ipa-4-1: 7a99f22ee0a4c5fd1d41722fa0101fee74e0c76e
ipa-4-0: 32b6eb5110b2f851fbedb39ac0d853b46087465f

Martin

Thanks, I will add tests next week.

--
Martin Basti

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


Re: [Freeipa-devel] [PATCH 0128] dnszone-remove-permission should raise NotFound if permission doesn't exist

2014-09-24 Thread Martin Kosek
On 09/24/2014 04:55 PM, Martin Basti wrote:
 Patch attached
 
 This probably should go to 4.0.x, 4.1 and master

It is obvious that this interface was designed this way. So you should
elaborate more on the should part, list use cases where current approach does
not work, link to tickets, ...


___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


Re: [Freeipa-devel] [PATCH 0128] dnszone-remove-permission should raise NotFound if permission doesn't exist

2014-09-24 Thread Martin Basti

On 24/09/14 17:30, Martin Kosek wrote:

On 09/24/2014 04:55 PM, Martin Basti wrote:

Patch attached

This probably should go to 4.0.x, 4.1 and master

It is obvious that this interface was designed this way. So you should
elaborate more on the should part, list use cases where current approach does
not work, link to tickets, ...



Sorry for that, I though I broke it during refactoring.

--
Martin Basti

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel