n00b question about code cleanup fixes

2014-11-05 Thread Derrick Schneider
Hey, all. I just downloaded and got Cloudstack building on my dev machine,
and I had a question about a minor code-cleanup issue I saw. Specifically,
in DatabaseUpgradeChecker, there's this:

if (Version.compare(trimmedCurrentVersion, upgrades[upgrades.length
- 1].getUpgradedVersion()) != 0) {
s_logger.error(The end upgrade version is actually at  +
upgrades[upgrades.length - 1].getUpgradedVersion() +
 but our management server code version is at  +
currentVersion);
throw new CloudRuntimeException(The end upgrade version is
actually at  + upgrades[upgrades.length - 1].getUpgradedVersion() +
 but our management server code version is at  +
currentVersion);
}


I thought I would clean this up to just have one copy of the string so that
one doesn't need to remember to update both copies when changing/expanding
the message. There are some other instances in the file as well.

This isn't a bug fix, but do I still file a bug report for tracking
requests? It's obviously not a feature, so it probably doesn't warrant
discussion (despite the fact that I'm emailing about it).

This is probably a n00b question, but it wasn't clear from the
documentation about contributing.


Re: n00b question about code cleanup fixes

2014-11-05 Thread Wido den Hollander


On 11/05/2014 07:28 PM, Derrick Schneider wrote:
 Hey, all. I just downloaded and got Cloudstack building on my dev machine,
 and I had a question about a minor code-cleanup issue I saw. Specifically,
 in DatabaseUpgradeChecker, there's this:
 
 if (Version.compare(trimmedCurrentVersion, upgrades[upgrades.length
 - 1].getUpgradedVersion()) != 0) {
 s_logger.error(The end upgrade version is actually at  +
 upgrades[upgrades.length - 1].getUpgradedVersion() +
  but our management server code version is at  +
 currentVersion);
 throw new CloudRuntimeException(The end upgrade version is
 actually at  + upgrades[upgrades.length - 1].getUpgradedVersion() +
  but our management server code version is at  +
 currentVersion);
 }
 
 
 I thought I would clean this up to just have one copy of the string so that
 one doesn't need to remember to update both copies when changing/expanding
 the message. There are some other instances in the file as well.
 
 This isn't a bug fix, but do I still file a bug report for tracking
 requests? It's obviously not a feature, so it probably doesn't warrant
 discussion (despite the fact that I'm emailing about it).
 

No, you don't need to. You can simply file a patch on reviews.apache.org
and it can go into cloudStack.

Always great to see commits coming from the community!

Wido

 This is probably a n00b question, but it wasn't clear from the
 documentation about contributing.
 


Re: n00b question about code cleanup fixes

2014-11-05 Thread Derrick Schneider
Thanks! Submitted. It's nothing major. Just something I noticed while I was
in that code.

On Wed, Nov 5, 2014 at 12:43 PM, Wido den Hollander w...@widodh.nl wrote:



 On 11/05/2014 07:28 PM, Derrick Schneider wrote:
  Hey, all. I just downloaded and got Cloudstack building on my dev
 machine,
  and I had a question about a minor code-cleanup issue I saw.
 Specifically,
  in DatabaseUpgradeChecker, there's this:
 
  if (Version.compare(trimmedCurrentVersion,
 upgrades[upgrades.length
  - 1].getUpgradedVersion()) != 0) {
  s_logger.error(The end upgrade version is actually at  +
  upgrades[upgrades.length - 1].getUpgradedVersion() +
   but our management server code version is at  +
  currentVersion);
  throw new CloudRuntimeException(The end upgrade version is
  actually at  + upgrades[upgrades.length - 1].getUpgradedVersion() +
   but our management server code version is at  +
  currentVersion);
  }
 
 
  I thought I would clean this up to just have one copy of the string so
 that
  one doesn't need to remember to update both copies when
 changing/expanding
  the message. There are some other instances in the file as well.
 
  This isn't a bug fix, but do I still file a bug report for tracking
  requests? It's obviously not a feature, so it probably doesn't warrant
  discussion (despite the fact that I'm emailing about it).
 

 No, you don't need to. You can simply file a patch on reviews.apache.org
 and it can go into cloudStack.

 Always great to see commits coming from the community!

 Wido

  This is probably a n00b question, but it wasn't clear from the
  documentation about contributing.