Thanks Tim.
Please see inline comment.
----- Original Message -----
From: Tim Quinn <Timothy.Quinn_at_Sun.COM>
Date: Saturday, November 3, 2007 1:58 pm
Subject: Re: Code Review Request
To: admin_at_glassfish.dev.java.net, Rajeshwar V Patil <Rajeshwar.Patil_at_Sun.COM>
> Hi, everyone.
>
> A few quick comments...and I hope these don't seem trivial because I
> don't think they really are; I think they help improve usability and
> user-friendliness.
>
> 1. I agree with Anissa's advice encouraging the use of a single
> message
> (and a single code path) instead of several of each to handle the
> various combinations of how many items of each type are available.
> Further, if you really want to include or exclude the "s" where
> appropriate, you can use this technique:
>
> msg.updatesAndNewModulesAvailable=The Update Center has {0}
> update{0,choice,0#s|1#|1<s} and {1} module{1,choice,0#s|1#|1<s}
> available for download to your system
>
> which gives results like this:
>
> The Update Center has 1 update and 1 module available for download to
>
> your system.
> The Update Center has 0 updates and 1 module available for download to
>
> your system.
> The Update Center has 2 updates and 3 modules available for download
> to
> your system.
> The Update Center has 1 update and 0 modules available for download to
>
> your system.
>
> As with Anissa's approach, all cases are handled using only one Java
> code path (no need to choose among several possible messages) and only
>
> one message format.
>
Previously, I thought of doing this but then opted to me more user friendly at the expense of more(complex) code.
>
> 2. The original message used the phrase "...and {1} new *software*..."
>
> The term "software" is not normally a noun that is quantifiable. That
>
> is, people don't normally say "I installed two softwares onto my
> system
> today." Far more often people use "software" as a singular noun
> referring to one or more programs, packages, etc., as in "Think of all
>
> the software Sun has produced this in its lifetime." Or as a modifier
>
> to some other noun, as in "I installed two software packages" or
> "...two
> software downloads." Or if it's clear, just leave out the word
> "software" and use another noun that is quantifiable. ("I installed
> two
> downloads today.")
>
> So in my suggested replacement message above I've used "module"
> instead
> of "software."
>
In UC we have tabs labeled, *Available Updates* and *Available Software*. In order to relate to those, we used the
term software instead of module.
>
> 3. In my suggestion I removed the word "new" because would there ever
> be
> "old" updates or downloads available?
>
Yes, we do have old updates and old software available. We are informing user only of *new* updates and *new* software available. In UC, new updates and new software are those updates and modules which user has never viewed before. Once user views them(by invoking UC), they become old and are never informed about. In UC, new updates and new software are marked with a *(star) icon.
>
> 4. Feel free to yell at me for expanding the scope, and I know I have
>
> not been involved in all the conversations about this topic. Since we
>
> are talking about the admin GUI, instead of reporting the message
>
> Please run the Update Center Client
> (install-root/updatecenter/bin/updatetool) for download and installation.
>
> how about if the GUI simply launches the UC client for the user (after
>
> asking if he or she wants to do so)?
Good Idea. I do not know whether its possible to address this for UR1 release.
>
>
> 5. If we don't want the GUI to launch the UC client, then at least the
>
> running code should pass the actual path to the install-root as an
> argument when formatting the "Please run the Update Center Client"
> message. That way the user does not have to figure it out himself or
>
> herself. It might be best to create a File object to the updatetool
> file and then displaying the canonical path in the message so we
> display
> the path according to the current platform's formatting (drive letter
> on
> Windows, / vs. \ for file separator, etc.).
>
> OK. I'm done suggesting more work!
I am not sure how much work is involved here. I will try to address this, if possible for this release.
Thanks
Rajeshwar
>
> - Tim
>
> Anissa Lam wrote:
> > Hi Rajeshwar,
> >
> > Thanks for taking care of the GUI module changes.
> > Just a comment, it will make the code much simpler if we use {0}
>
> > new update(s) instead of putting out different msg for 1 or
> > greater than 1 update. The same for modules.
> > If thats what you really want to do, then i see that there is
> missing
> > a case where update is more than 1 but Module is 1. You will
> then
> > need to add msg.updatesAndNewModuleAvailable in the Properties file.
> > So, either you simplify the code or add this missing case.
> >
> > thanks
> > Anissa
> >
> > Rajeshwar V Patil wrote:
> >> I am planning to check-in following changes to admin-gui module.
> >> Please review.
> >> These changes are related to UC issue-
> >> https://updatecenter.dev.java.net/issues/show_bug.cgi?id=336
> >>
> >> Brief context:
> >> ---------------
> >> Currently, on Admin GUI login, we report no. of updates available
> if
> >> any. Now, we also want to report number of new software available
> if
> >> any. UC code is already fixed. These are corresponding changes in
> >> Admin GUI.
> >>
> >>
> >> What the change is:
> >> ----------------------
> >> In addition to new updates, now we are also checking for new
> software
> >> modules and constructing the update message accordingly.
> >>
> >>
> >> Code diffs:
> >> ------------
> >>
> >> Index: handlers/CommonHandlers.java
> >> ===================================================================
> >> RCS file:
> >>
> /cvs/glassfish/admin-gui/src/java/com/sun/enterprise/tools/admingui/handlers/CommonHandlers.java,v
>
> >>
> >> retrieving revision 1.16
> >> diff -c -r1.16 CommonHandlers.java
> >> *** handlers/CommonHandlers.java 6 Jun 2007 08:07:34 -0000 1.16
> >> --- handlers/CommonHandlers.java 3 Nov 2007 17:39:28 -0000
> >> ***************
> >> *** 143,151 ****
> >> return;
> >> try{
> >> ! //We check if any thing is available for update once
>
> >> per session.
> >> ! int numModule =
> >> AMXUtil.getDomainRoot().getUpdateStatus().getNumModules();
> >> ! sessionMap.put("updateCenterMsg", (numModule > 0)?
> >> GuiUtil.getMessage("msg.updateAvailable") : "");
> >> }catch(Exception ex){
> >> //ex.printStackTrace(); was told that we shouldn't
>
> >> log update center exception
> >> sessionMap.put("updateCenterMsg", "");
> >> --- 143,151 ----
> >> return;
> >> try{
> >> ! //Check if any new updates or modules are available
> >> ! sessionMap.put("updateCenterMsg", getUpdateMessage());
> >> ! }catch(Exception ex){
> >> //ex.printStackTrace(); was told that we shouldn't
>
> >> log update center exception
> >> sessionMap.put("updateCenterMsg", "");
> >> ***************
> >> *** 628,633 ****
> >> --- 628,677 ----
> >> }
> >> } + private static String getUpdateMessage() {
> >> + //Get number of new updates available
> >> + int numUpdates =
> >> AMXUtil.getDomainRoot().getUpdateStatus().getNumModules();
> >> + + //Get number of new software available
> >> + int numNewModules =
> >> AMXUtil.getDomainRoot().getUpdateStatus().getNumNewSoftware();
> >> + + String updateMessage = "";
> >> + if ((numUpdates > 0) && (numNewModules > 0)) {
> >> + //New update as well as software modules are
> available
> >> + updateMessage = (numUpdates == 1) ?
> >> +
> >> GuiUtil.getMessage("msg.updateAndNewModulesAvailable",
> >> + new Object[]{numNewModules}) :
> >> +
> >> GuiUtil.getMessage("msg.updatesAndNewModulesAvailable",
> >> + new Object[]{numUpdates, numNewModules});
> >> + } else {
> >> + if ((numUpdates <= 0) && (numNewModules <= 0)) {
> >> + //No new update or sofware modules are available
> >> + updateMessage = "";
> >> + } else {
> >> + if (numUpdates > 0) {
> >> + //New updates are available
> >> + updateMessage = (numUpdates == 1) ?
> >> + GuiUtil.getMessage("msg.updateAvailable")
> :
> >> + GuiUtil.getMessage("msg.updatesAvailable",
> >> + new Object[]{numUpdates});
> >> + } else {
> >> + //New software modules are available
> >> + updateMessage = (numNewModules == 1) ?
> >> +
> >> GuiUtil.getMessage("msg.newModuleAvailable") :
> >> +
> >> GuiUtil.getMessage("msg.newModulesAvailable",
> >> + new Object[]{numNewModules});
> >> + }
> >> + } + }
> >> + + if(!updateMessage.equals("")) {
> >> + updateMessage = updateMessage + " " +
> >> + GuiUtil.getMessage("msg.runUpdateCenter");
> >> + }
> >> + + return updateMessage;
> >> + }
> >> + private static final String CHARTING_COOKIE_NAME =
> >> "as91-doCharting";
> >> private static final int INDEX=0;
> >> Index: resources/Strings.properties
> >> ===================================================================
> >> RCS file:
> >>
> /cvs/glassfish/admin-gui/src/java/com/sun/enterprise/tools/admingui/resources/Strings.properties,v
>
> >>
> >> retrieving revision 1.132.2.3.2.8
> >> diff -c -r1.132.2.3.2.8 Strings.properties
> >> *** resources/Strings.properties 30 Oct 2007 19:19:06 -0000
> >> 1.132.2.3.2.8
> >> --- resources/Strings.properties 3 Nov 2007 17:39:30 -0000
> >> ***************
> >> *** 228,234 ****
> >> msg.PingError=Ping returned error without known reason.
> >> msg.JmsPingSucceed=Ping succeeded: JMS service is running
> >> msg.Error=An error has occurred
> >> ! msg.updateAvailable=** Update is available. Please run the
> Update
> >> Center Client for download and installation. **
> >> msg.enableSuccessful=Selected application(s) has been enabled on
>
> >> all targets.
> >> msg.disableSuccessful=Selected application(s) has been disabled
> on
> >> all targets.
> >> msg.enableResourceSuccessful=Selected resource(s) has been
> enabled
> >> on all targets.
> >> --- 228,240 ----
> >> msg.PingError=Ping returned error without known reason.
> >> msg.JmsPingSucceed=Ping succeeded: JMS service is running
> >> msg.Error=An error has occurred
> >> ! msg.updatesAvailable={0} new updates are available.
> >> ! msg.updateAvailable=1 new update is available.
> >> ! msg.newModulesAvailable={0} new software are available.
> >> ! msg.newModuleAvailable=1 new software is available.
> >> ! msg.updatesAndNewModulesAvailable={0} new updates and {1} new
> >> software are available.
> >> ! msg.updateAndNewModulesAvailable=1 new update and {0} new
> software
> >> are available.
> >> ! msg.runUpdateCenter=Please run the Update Center Client
> >> (install-root/updatecenter/bin/updatetool) for download and
> >> installation.
> >> msg.enableSuccessful=Selected application(s) has been enabled on
>
> >> all targets.
> >> msg.disableSuccessful=Selected application(s) has been disabled
> on
> >> all targets.
> >> msg.enableResourceSuccessful=Selected resource(s) has been
> enabled
> >> on all targets.
> >>
> >>
> >>
> >> Thanks
> >> Rajeshwar
> >>
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: admin-unsubscribe_at_glassfish.dev.java.net
> > For additional commands, e-mail: admin-help_at_glassfish.dev.java.net
> >