shrinkwrap / descriptors

ShrinkWrap subproject for creating Archive Descriptors
Apache License 2.0
26 stars 30 forks source link

Sd 21 #57

Open rbattenfeld opened 12 years ago

rbattenfeld commented 12 years ago

Hi Andrew and Aslak

I implemented a first version of descriptor readers. I tried not to change the existing descriptors. The changes are:

  1. The read-only descriptors are called DescriptorReader, e.g. WebAppDescriptorReader
  2. The descriptor readers are type safe as the mutual descriptors but I couldn't reuse the same types. I think this is ok. A user knows in advance which descriptors (mutual or read-only).
  3. I removed all service files. These files are now generated. This is possible because the metadata-parser allows also to specify the descriptor name.
  4. I wrote a small test case in the ejb-jar31 test folder to check the basics.

Let me know what you think.

Happy Easter Ralf

ALRubinger commented 12 years ago

Awesome to see some progress on this. Some comments:

1) Work to continue synchronized with upstream/SHRINKDESC-21 2) CI Job: https://shrinkwrap.ci.cloudbees.com/job/ShrinkWrap_Descriptors_upstream-SHRINKDESC-21/ 3) I'd been expecting the mutable view to extend from the immutable one. In other words: EjbJar31MutableDescriptor extends EjbJar31Descriptor. 4) The patch looks like a lot of reformatting is taking place, which makes it especially difficult in the generated code scenario to see what's really changed. Can we back out the refomatting so that the diffs are more true to the functionally-changed content?

This can fuel our discussions now set for: http://timeanddate.com/worldclock/meetingdetails.html?year=2012&month=4&day=9&hour=17&min=0&sec=0&p1=195&p2=197

rbattenfeld commented 12 years ago

Hi Andrew, I reformatted the ddJavaAll.xsd hoping that the this is now better.

Cheers, Ralf

ALRubinger commented 12 years ago

Feedback sought to move forward: https://community.jboss.org/thread/198374

rbattenfeld commented 12 years ago

Hi Andrew

I synced my stuff with yours. I will continue with the API enhancements because this will makes it easier. What I will do is:

Already done is to pass a 'class' as argument.

Regards, Ralf

ALRubinger commented 12 years ago

Cool, I see that this is rebased of upstream/SHRINKDESC-21.

Some questions:

1) Why the source changes to metadata-parser? Should this be part of another issue? The end goal is to change the generation strategy sure, but first we need to agree on the final form in the API prototyping being done. 2) If we're making an object model to represent hierarchical data, how would we do this without "up()"?

With those addressed, we can likely squash your commits into one and put it into upstream/SHRINKDESC-21.

Looking forward to making even more progress over this in person at JUDCon and JBoss World!

S, ALR

rbattenfeld commented 12 years ago

Sorry that I was not precise enough. API changes must be agreed. I just want to discover possible solutions for problems reported by others. So, the existing API didn't change at all.

Question 1: These changes should allow that sequences of elements within a class can be passed in one call. When I mean sequences then I mean xsd:sequences which clearly says 'enter the following arguments in the this order'.

Question 2: The only other solution for up() is perhaps the standard new() operator. I think JAXB does it in the same way. Again, this is just an idea.

I will certainly not change the API without agreement and will also work on the read-only feature.

I hope this clarifies a little bit. And yes, I am looking forward as well to make progress at JUDCon and JBoss World. Ralf

rbattenfeld commented 12 years ago

I investigated some possible API enhancements which can be realized out of the generation step. The changes are just add-ons. The existing API is not changed at all. I will write a comment in the forum about this,

I will now investigate the read-only aspect further, so that we have a good base next week for a deeper discussion.

Regards, Ralf

rbattenfeld commented 12 years ago

Hi Andrew

I synced your POC. I am not so happy because I realized that the idea I had introduces a new layer. The current state is:

  1. The descriptors are based on your POC
  2. The filter element in your POC is replaced by the root element because this is the only valid element that can have a ref to the descriptor.
  3. The other elements are as before. I didn't had time to go further in my holidays:-)

I think, the state is good enough for the discussion.

See you tomorrow!