Hi Christian, I'm not a big fan of xml when dealing with not-very-complicated data. The data in startup.properties is just about the right complexity for a properties file. A feature repo is too much: it can contain more than one feature, and the name of the startup feature has to be hard coded.
Furthermore, now that the subsystem spec is fairly final I think we should look towards using spec features as much as possible and start thinking of karaf features as possibly obsolete. Pushing the karaf feature xml format into the framework startup is exactly opposite of this goal. I looked back and reviewed your patch. Mostly I'm impressed with how much you've contributed in the last few months :-) I wish I had as much time to spend on karaf.... Your patch is indeed pretty simple and small but my objections are really to the idea of using xml during startup rather than the implementation. I might have missed it but didn't think you addressed the question of why xml was better than a properties file. My views might not be shared by many others, for instance I really think we should try hard to make karaf runtime independent of maven including the mvn url handler, and I'm not sure anyone else agrees with me. thanks david jencks On Jun 2, 2012, at 2:17 AM, Christian Schneider wrote: > After my rather emotional driven first answer I would like to elaborate a bit > more. > > I have put a lot of effort into my implementation of feature reading in the > main. I made sure the code does not introduce any new dependencies which was > one of the issues. I added a test which shows the code works and also works > together with the startup.properties handling. The code was only about 20 > lines so it did not make the Main class really more complex. > > Guillaume correctly listed some unaddressed concerns. So lets look into the > remaining issues inline: > > Am 25.05.2012 17:55, schrieb Christian Schneider: >> Thanks for the explanations. >> >> I don´t think I can address these concerns while keeping the original idea >> of using features like the startup.properties. So I closed the issue as wont >> fix. >> >> Christian >> >> Am 25.05.2012 17:34, schrieb Guillaume Nodet: >>> David said he didn't want xml processing in the bootstrap. This >>> clearly has not been addressed. > As features are expressed in xml I can not really address this without > completely skipping the feature reading. This was the main reason why I > closed the issue. I hoped my simple implementation convinced David that xml > handling is not so bad after all. As he did not express any concerns about my > patch I thought this is resolved. >>> Andreas said there's no value in your change because the file is now >>> fully generated. This hasn't been addressed. > The value is in not having to generate the startup.properties. This would > make the karaf maven plugin simpler as the code could be removed there. This > is of course just my personal opinion. So I am not sure how to resolve that > concern. >>> I said only supporting a subset of features schema is a problem. This >>> hasn't been addressed. > I answered in the issue that the subset of a feature that is handled is quite > equivalent in what we can do in the startup.properties file. As the framwork > feature is the core of karaf I think we can > live with not having the ability to reference other features or define > configs. These additional features would make the feature handling too > complex for the Main class. So I think we can argue that it is necessary and > not a big problem to only support a subset. >>> >>> >>> I may have missed something. But when people disagree, letting time >>> pass does not usually change things. >>> AFAIK, those concerns has been raised on the patch you uploaded, so >>> there's something wrong here. > There was only one comment on the patch from Guillaume. It was the last one > in the list about not handling all possibilities of a feature. As I thought > this was not a severe issue and adddtionall features could be added later if > necessary I committed the patch. No one else commented or reviewed the patch. > So I thought this was a kind of silent approval. So I really had a good > feeling about the commit and was very disappointed about Guillaumes -1 as I > thought I really did everything as well as I could by supplying a patch and > waiting more than 4 days. > > @Guillaume so what is your proposal. How should I have handled this issue? > Should I have given the jira issue more time so more people could review the > patch? Should I ask for positive approval which is rather uncommon at apache? > > Christian > > > -- > > Christian Schneider > http://www.liquid-reality.de > > Open Source Architect > Talend Application Integration Division http://www.talend.com >
