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

Repeatable annotation for the connection factory and destination definition annotations - fix for #151 #298

Merged
merged 1 commit into from
Jun 29, 2021

Conversation

nderwin
Copy link
Contributor

@nderwin nderwin commented Jun 27, 2021

Signed-off-by:Nathan Erwin nathan.d.erwin@gmail.com

…stination definition annotations

Signed-off-by:Nathan Erwin <nathan.d.erwin@gmail.com>
@OndroMih
Copy link
Contributor

It really looks this is all that's needed. There's no mention about these annotations in the spec document.

@dblevins
Copy link
Contributor

Note we do have TCK updates to do when modifying APIs:

  • The signature files will need to be regenerated so the signature tests don't fail.
  • We should also add a TCK test that uses the annotation repeatably and verifies the vendor supports it. For that we can most likely find the test that uses JMSDestinationDefinitions and copy/modify it (hopefully there is a test that uses that annotation meaningfully).

@nderwin
Copy link
Contributor Author

nderwin commented Jun 29, 2021

@dblevins If you'll point me at the TCK, I can look into making a PR there too. I'd feel better having a test for this there too.

@nderwin nderwin deleted the feature/issue-151 branch June 29, 2021 21:37
@OndroMih
Copy link
Contributor

@nderwin, with the TCK it's not that easy. The Messaging TCK is a part of the Jakarta EE TCK. We're going to separate it away into the Messaging project, either into this repository or into a new repository. I think it's better to wait until we do it, as we may also modernize the TCK so that it's easier to work with it. The current TCK is based on the Java Harness test framework, which is not very convenient nowadays.

@dblevins
Copy link
Contributor

@nderwin Sounds great, Nathan. Thanks for volunteering! Fair warning the TCK is still one large suite covering all (most) specs, so it will be hard to digest and if you get anywhere you'll be one of only a handful who can grok it. That said, it's what we got, LOL :)

Here is the section where the JMS tests are located:

The best resource for help:

Here's a similar PR for when we made EJBs @Schedule annotation repeatable.

Many of us are hoping we can move all those tests into a new messaging-tck repo here, but so few of us really understand the TCK at that level, so it's just hopes and dreams at this point.

@dblevins
Copy link
Contributor

dblevins commented Jun 29, 2021

I think it's better to wait until we do it, as we may also modernize the TCK so that it's easier to work with it. The current TCK is based on the Java Harness test framework, which is not very convenient nowadays.

My gut instinct is we don't have the luxury to wait till after it's split out to do TCK updates. There's a significant chance splitting it out may not happen. The biggest blocker is there are so few people who understand the TCK harness. I think the more people that are brave enough to dig in as it is now, the higher the chance we can actually get it done.

@OndroMih OndroMih changed the title Possible fix for #151 Repeatable annotation for the connection factory and destination definition annotations - fix for #151 Jan 12, 2022
@arjantijms arjantijms added this to the 3.1.0 milestone Jan 24, 2022
@arjantijms arjantijms added the 3.1 label Jan 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants