You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by Derrick Schneider <de...@opower.com> on 2014/11/05 19:28:23 UTC
n00b question about "code cleanup" fixes
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
Posted by Derrick Schneider <de...@opower.com>.
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 <wi...@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.
> >
>
Re: n00b question about "code cleanup" fixes
Posted by Wido den Hollander <wi...@widodh.nl>.
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.
>