Skip to content

Small fixes and improvements to the reference documentation #29694

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

Closed

Conversation

simonbasle
Copy link
Contributor

@simonbasle simonbasle commented Dec 15, 2022

This PR fixes a few obvious typos and small issues in the reference documentation:

  • Remove mention that users could be on Java < 5
  • Fix typo 'can can'
  • Fix subsection style in WebFlux Concurrency Model
  • Fix typo: Inn -> In
  • Fix links to Jakarta Mail 1.6
  • Remove ref to JOTM, inactive since 2009

It makes 2 larger changes to improve cross-linking consistency between the MVC and WebFlux chapters:

  1. Turn plain 'WebFlux' links to 'See equivalent in the Reactive stack' and turn plain 'WebMVC' links to 'See equivalent in the Servlet stack'

I found the links between chapters to be a little bit too terse, and there were some inconsistencies

  1. Rework linking to Sprinv MVC Async support vs WebFlux section

These links point to a section in the same chapter, but could be confused with the "WebFlux" links from above

The PR has been split into meaningful commits for ease of review, but these can be squashed upon merging.

The link would be named "Compared to WebFlux", which is easy to mix up
with the various links to equivalent sections in the WebFlux chapter.
Here the links point to a small section comparing the Servlet Async API
to the WebFlux stack from a high perspective.

In this commit we eliminate most of these links, except at the beginning
of the Asynchronous section. We also add a small mention of the Servlet
configuration in the comparison paragraphs, as Configuring section is
the one furthest from the comparison paragraphs that used to have a link
to it.
The block title style previously used was not rendered in HTML and the
title wouldn't be differentiated from the text. It was in the PDF, as
italics.

Introducing delimited blocks in the open (`--`) style did introduce
styling, but the vertical alignment isn't great.

This commit turns these block titles to actual (deep) section titles.
In the final HTML, at this depth there is no numbering but bold styling
is there. The PDF rendering has also be verified to have relevant style.
@simonbasle simonbasle added the type: documentation A documentation task label Dec 15, 2022
@sbrannen sbrannen self-assigned this Dec 16, 2022
@sbrannen sbrannen added this to the 6.0.4 milestone Dec 16, 2022

This library is freely available on the web -- for example, in Maven Central as
`com.sun.mail:jakarta.mail`. Please make sure to use the latest 1.6.x version
rather than Jakarta Mail 2.0 (which comes with a different package namespace).
See the `v1.x` code in the https://github.com/jakartaee/mail-api/tree/v1.x[Jakarta Mail-Api repository].
Copy link
Member

Choose a reason for hiding this comment

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

Unless I'm missing something, this whole section is wrong regarding the versions.

We currently compile against jakarta.mail:jakarta.mail-api:2.0.1 for our e-mail support (pulling in dependencies on things like jakarta.mail.Message). So users of Spring Framework 6.0 must use Jakarta Mail 2.0+.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

eh, I made this change on the basis that the 1.6.x mention was correct, primarily because the link was 404. perhaps this needs to be evaluated / reworked separately then ? (with a specific backport to 5.3.x that looks like the edits I made)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've pushed a commit that removes that github link. So the content referring 1.6 is still probably wrong, but at least the Jakarta Mail link is fixed.

Copy link
Member

Choose a reason for hiding this comment

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

I made this change on the basis that the 1.6.x mention was correct,

Yes, I assumed that was the case. This is all coming from the 5.3.x version of the docs and was just an oversight (that it hasn't yet been updated in the 6.0.x docs).

I didn't mean to imply that you made an error with the versions. Rather, I just happened to notice it, because you modified that section. 😉

perhaps this needs to be evaluated / reworked separately then ? (with a specific backport to 5.3.x that looks like the edits I made)

Yes. 👍

Would you like to handle that? If not, I can take care of it.

Copy link
Member

Choose a reason for hiding this comment

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

Actually, I'll just go ahead and address that.

@simonbasle simonbasle requested a review from sbrannen December 16, 2022 16:18
sbrannen pushed a commit to sbrannen/spring-framework that referenced this pull request Dec 17, 2022
sbrannen pushed a commit to sbrannen/spring-framework that referenced this pull request Dec 17, 2022
sbrannen pushed a commit to sbrannen/spring-framework that referenced this pull request Dec 17, 2022
sbrannen pushed a commit to sbrannen/spring-framework that referenced this pull request Dec 17, 2022
The link was previously named "Compared to WebFlux", which is easy to
mix up with the various links to equivalent sections in the WebFlux
chapter. Here the links point to a small section comparing the Servlet
Async API to the WebFlux stack from a high perspective.

In this commit we eliminate most of these links, except at the
beginning of the Asynchronous section. We also add a small mention of
the Servlet configuration in the comparison paragraphs, since the
Configuring section is the one furthest from the comparison paragraphs
that used to have a link to it.

Closes spring-projectsgh-29694
sbrannen pushed a commit to sbrannen/spring-framework that referenced this pull request Dec 17, 2022
The block title style previously used was not rendered in HTML and the
title couldn't be differentiated from the text. Though, it was in the
PDF, as italics.

Introducing delimited blocks in the open (`--`) style did introduce
styling, but the vertical alignment isn't great.

This commit turns these block titles to actual (deep) section titles.
In the final HTML, at this depth there is no numbering but bold styling
is there. The PDF rendering has also been verified to have relevant
style.

Closes spring-projectsgh-29694
@sbrannen sbrannen closed this in 1a839f5 Dec 17, 2022
sbrannen added a commit to sbrannen/spring-framework that referenced this pull request Dec 17, 2022
sbrannen pushed a commit to sbrannen/spring-framework that referenced this pull request Dec 17, 2022
@sbrannen
Copy link
Member

This work has been merged into main (with partial commit squashing, reordering, etc.).

Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: documentation A documentation task
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants