> On Nov. 6, 2014, 4:49 a.m., Rajani Karuturi wrote: > > looking at the diff isnt giving me much information. Can you share what > > issues it is solving(a JIRA ticket?)? Is this related to internationalizing > > the strings? > > Derrick Schneider wrote: > Sorry it's not more clear. I asked on the dev list if this kind of change > needed a JIRA ticket, and my understanding was that it did not. If I > misunderstood, I'm happy to add one. I noticed this while debugging a > separate unrelated issue. > > This is just making code maintenance a little easier. Right now, the code > does this: > > doSomethingWithString("blah blah blah"); > doSomethingElseWithString("blah blah blah"); > > If you wanted to change the string, you'd have to change it in both > places, and thus you might forget one, which would create inconsistent log > messages relative to the message in the exception that gets thrown. > > So my change just moves the String into a variable and then uses that > variable in both places. Now changing the string means changing it once. And > it does it for a few places where that was true. > > I hope that clarifies.
I didnt check in my earlier review that the strings is being used twice. Thanks. I will submit the patch. - Rajani ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27652/#review60111 ----------------------------------------------------------- On Nov. 6, 2014, 12:46 a.m., Derrick Schneider wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/27652/ > ----------------------------------------------------------- > > (Updated Nov. 6, 2014, 12:46 a.m.) > > > Review request for cloudstack. > > > Repository: cloudstack-git > > > Description > ------- > > Takes strings that were duplicated within DatabaseUpgradeChecker and puts > them into nearby variables. > > > Diffs > ----- > > engine/schema/src/com/cloud/upgrade/DatabaseUpgradeChecker.java cba6b83 > > Diff: https://reviews.apache.org/r/27652/diff/ > > > Testing > ------- > > As this is just putting an extant value into a variable, I simply did a build > to make sure everything was still good. If there's other stuff I can do, > please advise. > > > Thanks, > > Derrick Schneider > >