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

Fix TCK JavaDoc #243

Merged
merged 3 commits into from
Jun 30, 2022
Merged

Fix TCK JavaDoc #243

merged 3 commits into from
Jun 30, 2022

Conversation

aubi
Copy link
Contributor

@aubi aubi commented Jun 28, 2022

Fix javadoc issues.

@aubi aubi marked this pull request as ready for review June 28, 2022 18:12
@aubi aubi changed the title Fix tck javadoc Fix TCK JavaDoc Jun 28, 2022
Copy link
Contributor

@KyleAure KyleAure left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Javadoc changes look good. I'm not sure why we would need to include Transaction as an API dependency. Perhaps others could chime in on that.

@smillidge
Copy link
Contributor

I think it is because it is referenced in the javadoc and now we use modules it is required. @aubi may have more info

@smillidge smillidge requested a review from njr-11 June 29, 2022 17:08
Copy link
Contributor

@njr-11 njr-11 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it is because it is referenced in the javadoc and now we use modules it is required. @aubi may have more info

It's actually API JavaDoc rather than TCK JavaDoc that references jakarta.transaction, and it's likely my fault from changes introduced in 3.0. In ContextServiceDefinition JavaDoc, it appears that I used {@link jakarta.transaction.UserTransaction} rather than {@code jakarta.transaction.UserTransaction} which is used in the JavaDoc of older code.

You might be able to avoid adding the dependency if you switch that reference.

@arjantijms
Copy link
Contributor

@njr-11 in some of the other specifications we have indeed replaced (or reverted) to using code instead of link. The letter sounds nice in theory, but in practice is unfortunately very troublesome. Maven, let alone java modules, don't have anything like a "doc scope" unfortunately.

Copy link
Contributor

@arjantijms arjantijms left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Transaction requirement should be removed, and we should revert back to code for the javadoc.

@aubi
Copy link
Contributor Author

aubi commented Jun 30, 2022

@njr-11 @arjantijms @KyleAure @smillidge
Yes, the dependency was because of javadoc link. I changed it to code and removed the dependency on transactions.
I tidied up the commits, so only javadoc fixes and build details remain. Transaction commits were removed.

Copy link
Contributor

@arjantijms arjantijms left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thx!

@aubi aubi merged commit 03087f5 into jakartaee:master Jun 30, 2022
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.

5 participants