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

WELDX-179 introduce @Qualified annotation #25

Closed
wants to merge 4 commits into from

Conversation

mojavelinux
Copy link
Member

Additionally made some optimizations in CoreExtension and added Properties#isProperty(Method)

- introduce the @qualified annotation to be used with @nAmed to assign a qualified name to bean
- add tests
- introduce default constructor for NamedLiteral
- add method to Properties to check if method is an accessor method
@pmuir
Copy link
Contributor

pmuir commented Oct 19, 2010

Qualify is a term already used in cdi, so I think we need to use another word, perhaps a synonym.

@mojavelinux
Copy link
Member Author

True, qualify is used in CDI. On the other hand, "fully-qualified class name" is a well known term in Java.

Regardless, I can offer some other ideas:

I've also added support for a target namespace via a class reference (the package of that target class). I figured this would be useful for people that want to use a fixed top-level namespace despite having subpackages.

@pmuir
Copy link
Contributor

pmuir commented Oct 19, 2010

Maybe @fullyqualified?

@pmuir
Copy link
Contributor

pmuir commented Oct 19, 2010

Also can you split out changes for this issue vs structural changes to the class. Additionally, we should support @qualified (or whatever it becomes) on packages.

- refactor @qualified to @fullyqualified
- honor @fullyqualified at the package-level (fully)
- add test for @fullyqualified at package level
- add more API docs to @fullyqualified
@mojavelinux
Copy link
Member Author

I had support on packages in one of the previous commits, but only on packages that also had @nAmed. I added support for it at the package level independent of @nAmed at the package level.

Just refactored to use @fullyqualified. Seems to be a nice fit to me.

I don't mean to sound like I'm grumbling, but the main reason there are structural changes was to make this easier to code. If I have to separate the two, then I need to do the work twice. If you think that's really necessary, I'll bite my lip and just do it.

@mojavelinux
Copy link
Member Author

Since we seem to be rolling with this feature, I'll go ahead and add the section to the docs, mostly from the api doc I already wrote.

@pmuir
Copy link
Contributor

pmuir commented Oct 20, 2010

No you wouldn't, just do the structural changes in commit #1 and the rest in commit #2

@pmuir
Copy link
Contributor

pmuir commented Oct 27, 2010

Rebased to one commit and closed.

@mojavelinux
Copy link
Member Author

Pete, sorry for not getting a chance to fix the commits as you request. I've been meaning to get to it, but I've just been short on time. Thanks for merging it in.

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
2 participants