Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Shrinkdesc 38 - Descriptor.getDescriptorName #14

Closed
wants to merge 1 commit into from

Conversation

aslakknutsen
Copy link
Member

I've added Descriptor name to the Descriptor interface, but how it is implemented makes me wonder..

Each time we go down a level in the Descriptor, e.g web.servlet().name() we create a new instance of the lower levels Desc object for so to pass along the current levels state down:

return new ServletDesc(getDescriptorName() /* main descriptor name /, getRootNode() / parent node /, servletNode / new current node /*)

(getRootNode we could infer from servletNode.parent in the constructor of the child.)

The impl works as is, but what i'm trying to say is.. it won't scale and seems a bit awkward. We don't have this issue in SW since we only have one level wrapped and always refer to the same Archive in the end.

Not sure which other Descriptor 'global' metadata like Name we need, but if we get more we should consider wrapping some of these object in some other way..

@ALRubinger
Copy link
Member

I'm not worried about the levels issue for SD-38; I've known this has been coming down the pipes for quite some time, but it's unrelated here. :)

What we need is a separate mechanism to navigate back "up" the tree to a parent. But again, that's another thing. I like this commit; pushing upstream; thanks.

This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants