[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [List Home]
Re: [virgo-dev] PDE2Virgo Plugin


On Wed, Jan 13, 2016 at 3:13 PM, Daniel Marthaler <DMarthaler@xxxxxxx> wrote:
Sorry for my delayed reply.

No problem, thank you very much for your time, your review and your comments.
ÂÂ
I went over your code changes on featue/329198 branch and so far from what I have seen they look ok. Some methods (e.g.: org.eclipse.virgo.ide.pde.core.internal.Builder#buildFilesInFolder()) look a bit huge at first glance. These could be split up for better readability and understandability.
Â

Noted, I'll try to improve it as you suggested. Any other comment about the code? Did you have a look at the new project wizard and at the new page of the runtime wizard as well (which also updates the Virgo repository properties file)?
Did you also try to create and run a Web project?

Finally, did you notice the new section in the server editor to reload or edit the associated target platform definition?
If so, do you think that a context menu for reloading/editing the target platform on the Server View would be useful (in addition to the section in the Server Editor) ?

I also installed build #6 from virgo.ide.snapshot.on.branch ci-job, created a server runtime together with a target platform. This worked well although, I first tried 3.7.0 virgo-kernel build which seems not to be yet supported by the tooling. I then downloaded 3.6.4 virgo-kernel and created a new "PDE Bundle Project", also this worked well.

Support for 3.7 is not yet ready in the tooling.

While investigating how to change the tools for 3.7, Florian and I found out that there are some utility bundles for working with the server that are included as an embedded binary JAR in the Virgo Tools plugins and that need to be updated for 3.7 as well. They are contained in a different source code repository that I had never checked out as I thought it belonged to the runtime group of repositories.

Support for Virgo 3.7 is now being implemented in a different branch; the PDE support branch and the Virgo 3.7 support branch will both be merged into master when they are considered good enough :-).
Â
I also tried to add an OSGI-INF folder, but here I was also required to add it to the Bundle-Classpath header, otherwise the tooling would just ignore it and this folder is not deployed to the satging directory. Should I file a bug for this?

Given that this is new development of a yet to be released feature, I would not open a bug for this. I'll just fix it as soon as possible in the branch. Probably not this week, as I am very very busy at work and very very tired in the evening. FYI, I used to create my OSGi-INF folders inside one of the source folders, but I can imagine PDE users like you to expect such folder to be a top-level folder (sibling of META-INF) therefore it's certainly better to have it supported by the builder.
ÂÂ
What I also found is that when I open the Server Editor for Virgo and activate both check boxes (Tail applicationtrace files into console view & Start server with -clean option) under "Server Startup Configuration", nothing happens. The server is not started using -clean option nor the application trace files are appended to console view. Did you also exprierenced this when you use the tooling?

No, I did not make any (at least intentional) change in that area. I'll try to replicate the problem and to figure out how to fix it.

Last question for you if you still have time to answer. Did you have any trouble installing the build in Eclipse? I was having some troubles with Mars. May I ask you how did you manage to install the build in Mars?

--
Gian Maria Romanato
<gm.romanato (at) gmail (dot) com>