cloudstack-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From John Burwell <>
Subject Re: fixPath (was: committer wanted for review)
Date Thu, 20 Jun 2013 22:07:05 GMT

As I mentioned in a previous email, it feels like a good place for Path value object that
encapsulates behavior.  I propose we throw this topic on the board for Sunday's Storage Architecture
session, and bring the resulting proposals back to the ML.

<rant>To my mind, we overuse String throughout the codebase when we should either be
using richer types provided by the Java runtime (e.g. or defining custom value
objects.  In addition to better levering the type checking of the compiler and potentially
exploiting polymorphism, rich value objects allow business rules to be neatly encapsulated
-- DRYing out the code base and allowing them to reliably unit tested.</rant>


On Jun 20, 2013, at 5:39 PM, Edison Su <> wrote:

>> -----Original Message-----
>> From: Daan Hoogland []
>> Sent: Thursday, June 20, 2013 1:03 PM
>> To: dev
>> Subject: Re: fixPath (was: committer wanted for review)
>> Yes, got it. It is quite big. It is dealing with more than just a path format, isn't
> Yah, I agree the patch changes too much(join path, and remove double quote, all over
the places).
>> On Thu, Jun 20, 2013 at 8:53 PM, Edison Su <> wrote:
>>> I uploaded the patch to dropbox:
>> bug14066.patch,
>>> can you access it? The patch is cooked by Fred Wittekind <
>>>> one year ago.
>>>> -----Original Message-----
>>>> From: Daan Hoogland []
>>>> Sent: Thursday, June 20, 2013 12:13 AM
>>>> To: dev
>>>> Subject: Re: fixPath (was: committer wanted for review)
>>>> I am not authorized to access the link you are sending Edison, but
>>> interested
>>>> in the contents. Could you send it please?
>>>> On Wed, Jun 19, 2013 at 10:26 PM, Edison Su <>
>>> wrote:
>>>>> The double slash can happen in every where, there is bug fix long
>>>>> time ago(, which
>>>>> tries to change all the path concatenation, unfortunately, it's
>>>>> not been checked into cloudstack.
>>>>> I am +1 with your patch, which at least fixes this particular
>>>>> issue, without changing too much code.
>>>>> From: Daan Hoogland []
>>>>> Sent: Wednesday, June 19, 2013 12:38 PM
>>>>> To: dev
>>>>> Subject: fixPath (was: committer wanted for review)
>>>>> John and others,
>>>>> I have been looking for the point where the wrong path is instantiated.
>>>>> After analyses I came to the DownloadAnswer class which contains
>>>>> the original fixPath method that I c&p'ed to do my thing. I cannot
>>>>> support this with logging however. Where the DownloadAnswer is
>>>>> created eludes me however. I got trapped between
>>>>> DownloadManagerImpl and
>>>> DownloadListener.
>>>>> The creation of the answer was as far as I could tell in
>>>>> handleDownloadProgressCmd but again I can not support this with
>>> logging.
>>>>> As the reason I suppect eclipse class-file caching but also this
>>>>> theory seems to not work.
>>>>> Anyone's got a good clue for me?
>>>>> I have burned to much time on this so I will save the patch for if
>>>>> it is locally needed by users, but I still want to find the best solution.
>>>>> As for John's argument of creating technical debt, I am now
>>>>> convinced that I am not adding but only using the present debt that is
>> there.
>>>>> The
>>>>> DownloadAnswer.fixup() method is doing the same on a more obscure
>>>>> place then my solution.
>>>>> Also, if we decide to apply my changes anyway I think we should up
>>>>> the loglevel at least as much as acceptable as to keep pointing to
>>>>> the technical debt that we have at the moment.
>>>>> On Tue, Jun 18, 2013 at 5:28 PM, Daan Hoogland
>>>>> < <>>
>> wrote:
>>>>> On 2013/6/18 16:49 , John Burwell wrote:
>>>>> Second, please don't take my feedback as passing judgements such
>>>>> as things being ugly or don't worry, I like the discussion and i
>>>>> don't mind losing an argument if the best solution arises from it.
>>>>> Let's see about that.
>>>>> --
>>>>> []<>

View raw message