You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@river.apache.org by Mark Brouwer <ma...@marbro.org> on 2008/05/17 16:02:21 UTC
Re: [jira] Updated: (RIVER-224) Problem using browser to destroy
service that supports DestroyAdmin but not JoinAdmin
Hi Bob,
All the changes for the service browser look fine to me (I integrated
them with my version of the browser and all runs fine).
I assume the diff you attached for the issues RIVER-224, 244, 210 and
217 is all the same. Just to nit-pick, I would prefer only one patch
that mentions that it incorporates multiple issues instead of giving
each issue the same patch.
Regards,
--
Mark Brouwer
Re: [jira] Updated: (RIVER-224) Problem using browser to destroy
service that supports DestroyAdmin but not JoinAdmin
Posted by Bob Scheifler <Bo...@Sun.COM>.
Mark Brouwer wrote:
> However disabling the popup for 'service removed' was the main
> motivation as bringing down a container with a huge number of services
> resulted in a lot of times clicking the OK button. Or did you mean
> attributes changed here?
I meant what I wrote, but OK, I've no objection to having a configuration
entry that applies to both.
> It is also possible to make a distinction between service removed and
> attributes changes, but personally I find that over the top.
OK.
- Bob
Re: [jira] Updated: (RIVER-224) Problem using browser to destroy
service that supports DestroyAdmin but not JoinAdmin
Posted by Mark Brouwer <ma...@marbro.org>.
Bob Scheifler wrote:
> Mark Brouwer wrote:
>> Another thing, as you have been working on the refresh part of the
>> service browser. I was always annoyed by having to click tons of times
>> on the YES button in case services were removed from the network, for
>> that purpose I added a property that allows you to auto accept the
>> modifications. I'm wondering whether I'm the only one annoyed by that
>> behavior?
>
> I think it would be fine to have a configuration entry (not a system
> property) to disable the popup for attribute refresh. Probably
> less desirable to apply it to the service removed popup.
You are right, I think that the system property predates Jini 2.x.
Besides a default in the Configuration I also can think of a menu entry
to set it.
However disabling the popup for 'service removed' was the main
motivation as bringing down a container with a huge number of services
resulted in a lot of times clicking the OK button. Or did you mean
attributes changed here?
It is also possible to make a distinction between service removed and
attributes changes, but personally I find that over the top. If people
have an opinion please let it know.
I'll file an issue soon. I also noticed there is no issue yet for
ServiceUI support in the service browser I'll file an issue for that too.
--
Mark
Re: [jira] Updated: (RIVER-224) Problem using browser to destroy
service that supports DestroyAdmin but not JoinAdmin
Posted by Bob Scheifler <Bo...@Sun.COM>.
Mark Brouwer wrote:
> Another thing, as you have been working on the refresh part of the
> service browser. I was always annoyed by having to click tons of times
> on the YES button in case services were removed from the network, for
> that purpose I added a property that allows you to auto accept the
> modifications. I'm wondering whether I'm the only one annoyed by that
> behavior?
I think it would be fine to have a configuration entry (not a system
property) to disable the popup for attribute refresh. Probably
less desirable to apply it to the service removed popup.
- Bob
Re: [jira] Updated: (RIVER-224) Problem using browser to destroy
service that supports DestroyAdmin but not JoinAdmin
Posted by Mark Brouwer <ma...@marbro.org>.
Bob Scheifler wrote:
>> All the changes for the service browser look fine to me (I integrated
>> them with my version of the browser and all runs fine).
>
> OK, thanks.
>
>> I assume the diff you attached for the issues RIVER-224, 244, 210 and
>> 217 is all the same.
>
> The patch for 224 is separate from the single patch for 210/217/244.
> I tried to indicate the multi-issue patch in the comments.
Oops missed that one, after having a look at 3 attachments and seeing
the same patch I thought anything related to the Service Browser was
covered by that single patch, looks like I should have checked the last
one (based on the descending ordering in my mail client).
BTW I applied the patch for 224 as well and looks fine. Maybe good to
mention that it also contains the feature that the service browser is
better in keeping an actual view on the lookup attributes in case a
service doesn't implement JoinAdmin or (my preference) you might want to
create an improvement issue for that, one more resolved issue in the
future release notes ;-)
Another thing, as you have been working on the refresh part of the
service browser. I was always annoyed by having to click tons of times
on the YES button in case services were removed from the network, for
that purpose I added a property that allows you to auto accept the
modifications. I'm wondering whether I'm the only one annoyed by that
behavior?
protected void receiveNotify(int transition) {
try {
if (Boolean.getBoolean("popup.ignore")) {
if (transition == ServiceRegistrar.TRANSITION_MATCH_NOMATCH)
cleanup();
else
refreshPanel();
return;
}
} catch (SecurityException e) {
// ignore
}
...
}
>> Just to nit-pick, I would prefer only one patch
>> that mentions that it incorporates multiple issues instead of giving
>> each issue the same patch.
>
> Where should I have put that one patch, and how should I have referred
> to it from the issues?
I was nit-picking and don't want to start a discussion but to give you
an answer. What I've done here before is to pick just one issue and
attach the patch there, mention it contains fixes for RIVER ..., etc.
just as you did. I don't see the need to update the other issues as
these will be updated once the aggregated patch is fine and checked in
subversion with the proper comments (I'm referring to the subversion tab
in JIRA). The benefits that I see is that it is less work for the
submitter, especially when patches have to be revised and that a reader
doesn't have to parse that many issue mails.
Also I decided to name my patch in a way that the RIVER issues it covers
is part of the name, such as RIVER-210-217-244.patch
--
Mark Brouwer
Re: [jira] Updated: (RIVER-224) Problem using browser to destroy
service that supports DestroyAdmin but not JoinAdmin
Posted by Bob Scheifler <Bo...@Sun.COM>.
> All the changes for the service browser look fine to me (I integrated
> them with my version of the browser and all runs fine).
OK, thanks.
> I assume the diff you attached for the issues RIVER-224, 244, 210 and
> 217 is all the same.
The patch for 224 is separate from the single patch for 210/217/244.
I tried to indicate the multi-issue patch in the comments.
> Just to nit-pick, I would prefer only one patch
> that mentions that it incorporates multiple issues instead of giving
> each issue the same patch.
Where should I have put that one patch, and how should I have referred
to it from the issues?
- Bob