cloudstack-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From terbolous <...@git.apache.org>
Subject [GitHub] cloudstack pull request: Embedded Tomcat & Jetty
Date Tue, 09 Jun 2015 07:25:42 GMT
Github user terbolous commented on the pull request:

    https://github.com/apache/cloudstack/pull/372#issuecomment-110257923
  
    I'm with Wilder & co on this.
    
    If you break it down, it's easier to test and thus merge.
    
    For instance, the mysql licensing could now potentially hold the complete
    PR, while tomcat/jetty changes are irrelevant and could've been merged (if
    tested and verified working).
    
    
    -- 
    Erik
    
    On Tue, Jun 9, 2015 at 9:10 AM, Rafael da Fonseca <notifications@github.com>
    wrote:
    
    > Hi @wilderrodrigues <https://github.com/wilderrodrigues> , @serverchief
    > <https://github.com/serverchief>
    >
    > Thank you for your feedback :)
    >
    > I understand your point, but since there are a lot of changes and most are
    > interconnected or change the same files, breaking it down would make it
    > take a long time to get things done and i'm used to working fast with a lot
    > of changes, sorry about that ;) . This is still not all the cleaning up i'd
    > like to do in packaging, and packing it all together makes it also a lot
    > faster to test for issues, than to do complete tests for a big bunch of PRs
    > individually, although a complete code review will be a bit taxing of
    > course, either way, reviewing 5x 200 lines of code, should be no faster
    > than to review 1000 line of code once, although testing packaging and
    > functionality once is a lot faster than doing it 5x, don't you agree?
    >
    > I will try to break down a some things in the coming days like i did with
    > the last packaging PR, but ultimately, the work goes a lot faster if i'm
    > not waiting for each individual change to get reviewed and merged before
    > moving on to the next step to improve the same thing. This is a particular
    > case because all the changes revolve around the same things: packaging and
    > distribution.
    > Either way, this allows for people to test once and make sure everything
    > is working.. if any issues are reported i'll be sure to fix them, and not
    > go back in time ;)
    >
    > I will also be sure to review all the changes myself and add anything
    > relevant i may have forgotten about, if any, to the initial comment to make
    > it easier to follow :)
    > Merging the whole PR would speed things up considerably and allow me to
    > continue improving packaging with a new base, but i'm not expecting to see
    > this through overnight of course :)
    > Ultimately it was easier and faster to get everything done in the same
    > branch and fix issues as they arose in my testing. This just gets it all
    > out there at once for the community to comment on and test as a whole.
    >
    > Other than this packaging PR which is something that needed an overhaul in
    > my opinion, I will try to keep my PRs as small as possible as you have
    > suggested.
    >
    > Cheers,
    > Rafael
    >
    > —
    > Reply to this email directly or view it on GitHub
    > <https://github.com/apache/cloudstack/pull/372#issuecomment-110255489>.
    >



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

Mime
View raw message