incubator-cloudstack-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Rohit Yadav <rohit.ya...@citrix.com>
Subject Re: Your commit d37ed18c3aff4fe522adb803322ba912819c73da placed scripts folder in a wrong place in master
Date Wed, 27 Feb 2013 05:49:06 GMT
Hi Min,

The issue was that Edison's patch was not on 4.1 and I started fixing
CLOUDSTACK-1244 on 4.1 which I fixed, then tried to cherry-pick on
master and got conflicts, and I saw Edison's patch on master. I just
moved the block back to its place (gave reference to his patch in
commit msg). The issue I found was since copying scripts depends on a
rule above it, what if in future someone changed the rule above it.
So, I made sure we explicitly make directories before copying, this
way I couple the mkdir and copy method. But either way it works, I've
no issues with that patch as well. Just that any such patch should go
in 4.1 that affects 4.1, the branches are already out of sync.

Thanks for you fix, cherry-picked on 4.1: 25efe050

Regards.

On Wed, Feb 27, 2013 at 4:42 AM, Min Chen <min.chen@citrix.com> wrote:
> Hi Rohit,
>
> In syncing with master with my feature branch vim51_win8, I filed this bug https://issues.apache.org/jira/browse/CLOUDSTACK-1387,
mvn build failed to copy scripts folder under "client/target/cloud-client-ui-4.2.0-SNAPSHOT/WEB-INF".
Edison checked in a fix to master and correctly addressed the problem. Today when I rebased
with master again, this is broken again due to your recent commit d37ed18c3aff4fe522adb803322ba912819c73da.
First, I noticed that you have reverted Edison's commit to explicitly create a directory,
any reason for that? By moving copying scripts after mvn copy, it can be guaranteed that the
directory already exists. Secondly, your fix is not correct, which will place a nested scripts
folder under "client/target/cloud-client-ui-4.2.0-SNAPSHOT/WEB-INF/scripts". To unblock myself
from testing vmware, I have checked in  cf06861c9bf883f2ca1ec818fe7bd4ad26b84318 to address
this issue.
>
> Thanks
> -min
>

Mime
View raw message