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
