cloudstack-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Chip Childers <chip.child...@sungard.com>
Subject Re: Reformatting UI code
Date Tue, 23 Jul 2013 16:36:11 GMT
On Tue, Jul 23, 2013 at 09:37:26AM -0600, Mike Tutkowski wrote:
> One other thought here is that we might want to wait until, say, the day
> after code freeze - if allowable - and then check in these kinds of changes
> (basically give them special permission to go in after code freeze).
> 
> I think they're valuable since they do make the code much more readable,
> but it might be better if they go in once development is "frozen."

The challenge with that, is that sweeping changes like this (or
architectural changes as well) are best done *early* in a release cycle.
The challenge we've run into here is that while 4.2 work is proceeding,
master is open for 4.3 changes (and there is a preference that if
something big is going to come in, nows the time to do it).

Perhaps that model is what's broken?

> 
> 
> On Tue, Jul 23, 2013 at 8:42 AM, Sebastien Goasguen <runseb@gmail.com>wrote:
> 
> >
> > 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
> > >>>>
> >
> >
> 
> 
> -- 
> *Mike Tutkowski*
> *Senior CloudStack Developer, SolidFire Inc.*
> e: mike.tutkowski@solidfire.com
> o: 303.746.7302
> Advancing the way the world uses the
> cloud<http://solidfire.com/solution/overview/?video=play>
> *™*

Mime
View raw message