cloudstack-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Sebastien Goasguen <run...@gmail.com>
Subject Re: Reformatting UI code
Date Tue, 23 Jul 2013 14:42:17 GMT

On Jul 22, 2013, at 6:54 PM, Jessica Wang <Jessica.Wang@citrix.com> wrote:

> Sebastien,
> 
> Brian and I are fix 4.2 bugs.
> Any fix for 4.2 bugs should go to both 4.2 branch and master branch.
> That's why Brian and I still need to merge our check-in between master branch and 4.2
branch.

Ok got it .

Ian next time you send a patch related to the UI maybe you can set Jessica and Brian as reviewers.

thanks,

> 
> Jessica
> 
> -----Original Message-----
> From: Sebastien Goasguen [mailto:runseb@gmail.com] 
> Sent: Monday, July 22, 2013 3:53 PM
> To: Brian Federle
> Cc: Jessica Wang; Pranav Saxena; Ian Duffy (ian@ianduffy.ie); dev@cloudstack.apache.org
> Subject: Re: Reformatting UI code
> 
> 
> 
> On 23 Jul 2013, at 00:34, Brian Federle <Brian.Federle@citrix.com> wrote:
> 
>> In this case it was fine, I could resolve the conflicts by git's -Xignore-space-change
and reformatting accordingly.
>> 
>> The main issue was that I didn't see any indication that this was being committed
from reading the discussion thread. Since we're doing a lot of bugfixing right now for 4.2,
a large sweeping commit like this should at least be posted as part of thread, so that there
is heads-up so that we can prepare any pending changes we have to avoid being blocked.
>> 
> 
> Ok but this is not for 4.2. It was committed to master. How is it impacting your 4.2
fixes ?
> 
> 
>> -Brian
>> 
>> -----Original Message-----
>> From: Sebastien Goasguen [mailto:runseb@gmail.com] 
>> Sent: Monday, July 22, 2013 3:26 PM
>> To: Jessica Wang
>> Cc: Pranav Saxena; Ian Duffy (ian@ianduffy.ie); dev@cloudstack.apache.org; Brian
Federle
>> Subject: Re: Reformatting UI code
>> 
>> Hi jessica,
>> 
>> Sorry this is causing you trouble. Ian us developing his ldap plugin in a feature
branch .
>> This was a UI cosmetic code change, i saw that pranav shipped it in RB but did not
apply the patch. Since the three if us work in more or less the same time zone i applied the
patch quickly to master. I did think about other UI work but since 4.2 is in feature freeze
i did not think committing to master would be a problem.
>> 
>> Moreover i did not know u were working on UI feature branches. Where can we look
at the feature descriptions ? 
>> 
>> I suppose u could revert the patch . But for features for 4.3 i think it should be
rebase of your feature branches . I dont think this change should be impacting 4.2 not sure
why u say thats the case.
>> 
>> Thoughts ?
>> 
>> -Sebastien
>> 
>> On 23 Jul 2013, at 00:07, Jessica Wang <Jessica.Wang@citrix.com> wrote:
>> 
>>> Pranav, Ian, Sebastien,
>>> 
>>> The problem is the duration between the time Ian brought it up on the mailing
list and the time Ian/Sebastien checked in the change to master branch is too short (less
than 4 hours).
>>> 
>>> Ian brought it up on the mailing list at Thu 7/18/2013 5:44 AM (email subject
is "Auto format javascript"). 
>>> Sebastien checked in Ian's change to master branch at 7/18/2013 9:34 AM (Commit
hash: ad69bc8da3244b783dd003ddf3184fca2762c514).
>>> 
>>> This is a big change of UI code.
>>> In GIT's view, every line in JS files has been changed (If you look at code difference
in GIT's history).
>>> GIT sees it as "delete all lines and add new different lines".
>>> I was unable to merge my check-in from master branch to 4.2 branch (or any other
branch) since GIT sees JS files in master branch and other branches are totally different.
>>> 
>>> Shouldn't this kind of big change be checked in to a different branch (not master
branch) first? Then, submit a merge request to community, wait for 72 hours, then merge to
master branch eventually?
>>> 
>>> Jessica
>>> 
>>> 
>>> -----Original Message-----
>>> From: Pranav Saxena [mailto:psbits@gmail.com]
>>> Sent: Monday, July 22, 2013 12:18 PM
>>> To: dev@cloudstack.apache.org; Brian Federle
>>> Cc: Ian Duffy (ian@ianduffy.ie)
>>> Subject: Re: Reformatting UI code
>>> 
>>> Hey Brian,
>>> 
>>> Sorry to hear that it caused merge conflicts for you . But Ian did 
>>> bring it up on the mailing list and I suggested him to use the js 
>>> beautifier tool for reformatting the js code which I guess you missed 
>>> probably because of the "heavy" traffic on the dev list and thereafter 
>>> Sebastien merged the code when the discussion and the reviews ended. 
>>> Anyways, I'll also try to ping you personally on such occasions in the 
>>> future to let you know if any major changes are being committed. Now , 
>>> probably you would need to do a lot of rebasing , sorry for that !!
>>> 
>>> Thanks,
>>> Pranav
>>> 
>>> 
>>> On Tue, Jul 23, 2013 at 12:29 AM, Jessica Wang <Jessica.Wang@citrix.com>wrote:
>>> 
>>>> +1
>>>> 
>>>> -----Original Message-----
>>>> From: Brian Federle [mailto:Brian.Federle@citrix.com]
>>>> Sent: Monday, July 22, 2013 11:33 AM
>>>> To: dev@cloudstack.apache.org
>>>> Cc: Ian Duffy (ian@ianduffy.ie)
>>>> Subject: Reformatting UI code
>>>> 
>>>> Hello,
>>>> 
>>>> Recently I discovered that all JS and UI code have been reformatted 
>>>> to 4
>>>> spaces:
>>>> 
>>>> commit ad69bc8da3244b783dd003ddf3184fca2762c514
>>>> Author: Ian Duffy <ian@ianduffy.ie>
>>>> Date:   Thu Jul 18 15:39:28 2013 +0100
>>>> 
>>>>  Format JS
>>>> 
>>>> While I do appreciate people coming in to help clean up the UI code, 
>>>> and don't mind if we change the indent level to be consistent with 
>>>> the rest of the code base, this commit is causing a lot of git 
>>>> conflicts with various development branches I'm working on. Please 
>>>> give a bit more heads up in the future about this, try to CC the main 
>>>> UI developers about it first before committing - right now myself and 
>>>> Jessica Wang (jessica.wang@citrix.com) do the majority of UI development.
>>>> 
>>>> Thanks,
>>>> Brian
>>>> 


Mime
View raw message