I added the hacking section https://wiki.openstack.org/wiki/Fuel#Hackingwith the content from https://etherpad.openstack.org/p/adding_fuel_lib_modules
On Mon, Apr 7, 2014 at 12:42 AM, Bogdan Dobrelya <[email protected]>wrote: > On 03/28/2014 11:21 PM, Andrew Woodward wrote: > > OK, we are slightly off topic here and I want to steer it back. > > > > The point isn't that we _will_ track upstream the point is that if we > > don't change we _cant_ track upstream and we _cant_ review large commits > > like this. > > > > This new processes wont give a damn about where you took from the > > upstream, It will leave a marker for others to be able to have a chance > > in hell of making a qualified review of the module and where you took it > > from as well as separately review you changes. > > > > Now what has occurred with https://review.openstack.org/#/c/80024/ is > > that Dimitry has reviewed it under the guise of > > > > 1. Create a review request with a verbatim copy of the upstream > > module and no other related modifications. This review should also > > contain the commit hash from the upstream repo in the commit > > message. The review should be looked over for reasons for rejecting > > the entire module, such as license issues. Forbearing such issues it > > should be accepted with out requiring modifications. > > > > > > Due to this review, Dimitry has found a good reason for "rejecting the > > entire module" as the point that module was pulled from has alot of > > wasted lines that the newer version replaces. It's now necessary to > > justify why we should except this crap over the newer and cleaner > > version upstream. > > > > Wile we should track and keep close to upstream or use only upstream, I > > know we need to take steps. This is why I only propose that we separate > > out the upstream module pull from your own development, it gives us: > > * improved view of what you want to add > > * improved view of what had to be done to "fuel" the module > > * a clear scope of review instead of blindly adding modules > > * prevents you from being the blame when the code clearly came from > upstream > > * review for really bad upstream modules (license, code quality, ...) > > * a chance (not requirement) to diff our changes from the upstream and > > propose changes back upstream > > > > For now we can just stick with improved review process. As we get better > > we can start working on upstream's more. > > > > So to re-iterate the rules I will update on our docs / wiki > > with https://etherpad.openstack.org/p/adding_fuel_lib_modules > > Please update the https://wiki.openstack.org/wiki/Fuel for appropriate > contribution workflow. For today it is still not clear how to contribute > to the Fuel projects, e.g. Puppet manifests for Fuel-library. > > Here is an another example of stuck for MongoDB support: > 1) We have a verbatim copy of upstream for puppetlabs-mongodb > https://review.openstack.org/#/c/85350/ > 2) We have a feature team changes for Fuel as a separate patch: > https://review.openstack.org/#/c/71901/ > 3) Our Ceilometer dev team is not aware of this discussion - probably > because of lack of documentation at the wiki pages. > > > > > > > > On Fri, Mar 28, 2014 at 12:41 PM, Dmitry Borodaenko > > <[email protected] <mailto:[email protected]>> wrote: > > > > No. The fact that we failed miserably at tracking upstream so far is > > not a valid excuse for refusing to even try: > > > http://hakanforss.files.wordpress.com/2014/03/are-you-too-busy-to-improve1.png > > > > Vladimir, you optimization goal is flawed: it assumes that staying > > close to upstream is a goal in itself. It is not a cult, there are > > important reasons why we want to track upstream. > > > > Yes, sometimes we find and fix bugs in the upstream code, but it is > > much more common for upstream to find and fix its own bugs. On > > average, we loose code quality over time, and that causes us to lose > > velocity by wasting efforts on fixing bugs that were already > addressed > > by upstream, or bugs that were introduced because our code is poorly > > structured and full of ugly hacks that upstream would have done (or > > already did) better. > > > > Staying close to upstream and submitting your fixes to them on a > > regular basis also makes it more likely that upstream developers > would > > be able and willing to help you. Right now we can't report bugs > > upstream (because we've diverged too far), we can't submit patches > > (often because we do things in a Fuel specific way that would be > > unacceptable to upstream), so we're always on our own when we have > > problems with any of the Puppet code in fuel-library. > > > > In short, assuming that we can do a better job alone than in > > collaboration with all our upstreams is just arrogant. We're not that > > good and there's not that many of us. > > > > Bogdan, your logstash commit is such an obvious illustration of the > > above that I am very surprised that I have to explain it. > > > > When I said that merging this patch will increase our Puppet code > base > > by 130% I expected everybody to understand that it is nothing short > of > > a catastrophe: > > > https://stackoverflow.com/questions/184071/when-if-ever-is-number-of-lines-of-code-a-useful-metric > > > > When I said that it looked auto-generated I thought it would be > > treated as a red flag to be verified, not just an observation that > can > > be ignored. You have to have very good reasons to commit > > auto-generated code (instead of just the generator), and > > puppet-logstash didn't have them. Even worse, it didn't even include > > the generator script (although commit history looks as if they kept > > using it after the initial commit). > > > > The fact that current version of puppet-logstash is able to do the > > same task in merely 673 lines of Puppet code instead of 20790 is by > > itself a MASSIVE improvement. If we commit the old version now, and > > give up on tracking upstream, we miss this improvement and are stuck > > maintaining an obsolete 20KLOC version for years. It will not be > > pleasant. If at some point we give up and try to move to a newer > > upstream version, we'll have to deal with the fact that upstream had > > to do a full rewrite of the manifests. And we would have to spend > even > > more effort migrating to the new upstream version in the future that > > we would now, because we'll have accumulated more Fuel specific > > changes that will have to be redone essentially from scratch. > > > > I don't see what better example of needing a newer upstream version > > you may need. There may be cases where it makes sense to postpone > > merging latest upstream, but this is certainly not one of them. > > Especially since it's not merged yet. > > > > > > On Fri, Mar 28, 2014 at 5:36 AM, Vladimir Kuklin > > <[email protected] <mailto:[email protected]>> wrote: > > > Guys, I suggest a hybrid approach. > > > > > > Let's just specify what do we want. Let me write these > > requirements down in > > > some kind of optimization problem > > > > > > We want to be as close to upstream modules as possible, subject to > > the fact > > > that our code should satisfy all the functional requirements for > > particular > > > features. > > > > > > In this case I would assume we need the following to do: > > > > > > 1) For any request require from the requester to submit the > > original module. > > > Not the current upstream HEAD, but his head where his own > development > > > branched from. > > > 2) Then the developer must submit the diverged code. As long as it > > meets > > > functional requirements, it should be OK to use it in FUEL > > > 3) Set upstream merging task as a "stretch goal" and strive to be > > as close > > > to upstream as possible. > > > > > > There should almost no cult of "upstream" code in our project, I > > think. > > > Because if there is a code piece that is not working in the > > upstream and we > > > fix it and upstream community does not accept it, then we have > > nothing to do > > > but maintain our own fork. Thus, let's split requests according to > > this > > > approach and request diverging changes. If they work - let's merge > > them and > > > leave upstream merging as background task. > > > > > > > > > On Fri, Mar 28, 2014 at 12:08 PM, Bogdan Dobrelya > > <[email protected] <mailto:[email protected]>> > > > wrote: > > >> > > >> On 03/19/2014 07:45 PM, Dmitry Borodaenko wrote: > > >> > On Wed, Mar 19, 2014 at 4:56 AM, Bogdan Dobrelya > > >> > <[email protected] <mailto:[email protected]>> wrote: > > >> >> On 03/19/2014 01:31 PM, Dmitry Nikishov wrote: > > >> >>> Actually, zabbix module doesn't come from any upstream repo, > > it's our > > >> >>> own reimplementation of the module by PL team, which was > based on > > >> >>> upstream code. In Zabbix thread it was suggested that we > > split it into > > >> >>> multiple commits instead of trying to push the whole thing at > > once. > > >> > > > >> > It can't be both our own reimplementation and based on upstream > > code > > >> > at the same time. If it's not rewritten from scratch, you > > should track > > >> > down the original upstream version and start your commit series > > from > > >> > that. > > >> > > > >> >>> It's unclear how it will affect internally developed modules > like > > >> >>> zabbix > > >> >>> one. Should there be any at all? Or should we make a public > > repo with > > >> >>> that module first and then try to include it into > fuel-library? > > >> > > > >> > That's a very good question. Personally, I think that we should > > prefer > > >> > the second option most of the time: create a public repo for the > > >> > standalone module, and use that as upstream for the copy of that > > >> > module in fuel-library. It will make it more likely that this > > module > > >> > will attract other engineers who will help us find and fix bugs > in > > >> > this code, and eventually even add more new features that we'd > > be able > > >> > to reuse in Fuel. > > >> > > > >> >> It is also not quite clear how to submit these team-specific > > changes. > > >> >> 1) E.g. I've submitted the puppet-logstash module from > > Apollo11 team > > >> >> (Poland) and that module was diverged from the original > upstream > > >> >> version > > >> >> by the team. Now we have a two diverged versions - an upstream > > one and > > >> >> a > > >> >> submitted one. > > >> > > > >> > I think that you should present each hand-over as a separate > > commit: > > >> > {original upstream commit} -> {modifications from Apollo11 > team} -> > > >> > {modifications to integrate with Fuel}. If you have the commit > > history > > >> > from Apollo11 and it's not too long, it would be nice to have > > all of > > >> > it instead of squashing it all into one commit (although if > > it's more > > >> > than a dozen commits it might now be worth the trouble of > > pulling each > > >> > commit through Gerrit). > > >> > > > >> >> 2) Should I submit the diverged commits rebased onto the > upstream > > >> >> HEAD/stable version as a separate patchset which depends on the > > >> >> verbatim > > >> >> copy of HEAD/stable patchset? That could be a very bad idea, > > because > > >> >> rebasing might broke the module completely. > > >> > > > >> > Yes, I also think that would be a bad idea. Rebase is a kind of > a > > >> > change, you don't want to combine that and other changes in a > > single > > >> > commit, or you loose ability to distinguish what were you > > changes and > > >> > what changed due to rebase. > > >> > > > >> >> 3) Should I do the same as (2) but use the common parent > > commit as an > > >> >> upstream base for verbatim copy instead? Despite on no more > > rebasing > > >> >> needed, that is not so good idea as well, because it would also > > >> >> complicate the upstream sync contribution process, if any > > planned in > > >> >> the future. > > >> > > > >> > You're not going to be able to kill two rabbits with one stone > > here. > > >> > Either you significantly diverge from upstream, and keeping up > > will be > > >> > near impossible, or you keep Fuel specific changes minimal and > well > > >> > isolated, and keeping up becomes simple. > > >> > > > >> > a) If you diverge, the best you can do is submit all your > > >> > non-Fuel-specific improvements to upstream (you will obviously > > need to > > >> > heavily modify them to decouple from Fuel specific code), and > then > > >> > periodically (e.g. once per Fuel release) merge upstream > > changes back > > >> > into Fuel by hand. The further you deviate, the harder this > process > > >> > becomes. It becomes even harder if you don't submit anything to > > >> > upstream, because there will be more changes to hand-port later. > > >> > > > >> > b) If you can isolate Fuel specific code, keeping up with > upstream > > >> > becomes much easier. Create a fork of upstream repo on Github, > > create > > >> > a fuel branch in that fork, commit all changes for that module > > to the > > >> > fuel branch before submitting them to fuel-library. Submit non > Fuel > > >> > specific changes to upstream (keep them on your fuel branch > > until they > > >> > are merged). Pulling a new upstream version into fuel-library > > becomes > > >> > a rebase of your fuel branch of the forked upstream onto the > latest > > >> > upstream release, and then copying the result verbatim into > > >> > fuel-library. If Fuel specific code is well isolated, that > > rebase will > > >> > be trivial. > > >> > > > >> >> 4) So, looks like the only good option is to accept changes to > the > > >> >> puppet modules which are only the sync requests from the > > upstream (see > > >> >> Openstack projects and Oslo) and never change them locally in > > the Fuel? > > >> >> But I'm afraid the Fuel puppet modules are not ready yet for > such > > >> >> dramatical changes... Looks like we need a kind of Fuel-oslo ;) > > >> > > > >> > I think we're very far from being able to use this approach. > > >> > > > >> > > >> Dmitry, thank you - that is a good point that makes sense. > > >> But looks like we still have a decision-blocker for introducing > > any new > > >> modules for puppet in Fuel-library, if one was not synced with the > > >> upstream while in dev / PoC process (i.e. we silently follow the > > >> rejected options (2) and (4) despite on the fact we have near to > > 90% of > > >> puppet modules in Fuel are far more than 1 year outdated and never > > >> synced with upstream) > > >> > > >> Here is an example of such decision blockers: > > >> https://review.openstack.org/#/c/80025/ > > >> https://review.openstack.org/#/c/80024/ > > >> > > >> Hence, lets accept we have a blocker condition in introducing > puppet > > >> modules for Fuel, lets decide how to deal with it once again and > > follow > > >> it in reviews as well. > > > > -- > > Dmitry Borodaenko > > > > -- > > Mailing list: https://launchpad.net/~fuel-dev > > Post to : [email protected] > > <mailto:[email protected]> > > Unsubscribe : https://launchpad.net/~fuel-dev > > More help : https://help.launchpad.net/ListHelp > > > > > > > > > > -- > > Andrew > > Mirantis > > Ceph community > > > > > > This body part will be downloaded on demand. > > > > > -- > Best regards, > Bogdan Dobrelya, > Skype #bogdando_at_yahoo.com > Irc #bogdando > -- Andrew Mirantis Ceph community
-- Mailing list: https://launchpad.net/~fuel-dev Post to : [email protected] Unsubscribe : https://launchpad.net/~fuel-dev More help : https://help.launchpad.net/ListHelp
