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

Polish reference documentation #220

Closed
wants to merge 21 commits into from

Conversation

yossisp
Copy link
Contributor

@yossisp yossisp commented Jun 24, 2023

  1. Generating application module canvases using Documenter - to be used before canvases generation code snippet.
  2. I think the author meant @DependsOn instead of @Order because @Order is used in a very specific case when the initialization order of Spring beans of the same type needs to be set.
  3. Classes implementing ApplicationModuleInitializer must implement initialize() method with public void initialize() signature.
  4. Fix org.springframework.modulith:spring-modulith-observability artifact (doesn't exist) to org.springframework.experimental:spring-modulith-observability.
  5. Add the default location of generated docs.

odrotbohm and others added 17 commits May 2, 2023 14:36
…c-… namespace in JDBC-based event support.
…ht profiles.

Remove the nesting of the test classes as they will be executed without profiles activated.
A few more comments. Test dependency instead of the core one to include the subsequently shown API.
…ependsOn can be used for beans of different types.

I think the author meant @dependsOn not @order annotation.
…oesn't exist. org.springframework.experimental:spring-modulith-observability was the intention.
@yossisp
Copy link
Contributor Author

yossisp commented Jun 27, 2023

The PR fails because of CI error, artifact not being pushed because of

Artifactory configured to accept only encrypted passwords but received a clear text password, getting the encrypted password can be done via the WebUI.

@odrotbohm odrotbohm self-assigned this Jul 6, 2023
src/docs/asciidoc/60-documentation.adoc Show resolved Hide resolved
@@ -58,7 +58,7 @@ ComponentB --> ComponentA

....

While developers could of course define the execution order via Spring's standard `@Order` annotation or `Ordered` interface, Spring Modulith provides an `ApplicationModuleInitializer` interface for beans to be run on application startup.
Copy link
Member

@odrotbohm odrotbohm Jul 6, 2023

Choose a reason for hiding this comment

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

Please scratch that. @DependsOn is not a recommended means for ordering user-facing components. @Order/Ordered is exactly what was meant.

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 reverted the wording but I have a question: @Order is mentioned in the context of Application Module Initializers which are intended to be used for different modules, not multiple implementations of the same module:

If a module B depends on module A, the initialization code of A has to run before the one for B, even if the initializers do not directly depend on another.

This is why I think the @Order parallel is confusing as it's intended to order multiple implementations of the same interface while initializers are intended for different modules altogether. Does this make sense?

Copy link
Member

Choose a reason for hiding this comment

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

I think what plays into this is the fact that for module-specific initialization, you usually want transactional behavior. That's not something you can get with simple components and an @PostConstruct callback in components you'd interconnect via @DependsOn.

In other words, you typically declare an interface for the initializer with a custom method that can be @Transactional and a central component that would depend on List<MyInitializer> and invoke those transactional methods. For that list to be ordered, you'd either use @Order/Ordered. Pretty similar to what we actually implement with ApplicationModuleInitializer.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@odrotbohm Got it, thank you for the explanation!

src/docs/asciidoc/80-observability.adoc Outdated Show resolved Hide resolved
@odrotbohm odrotbohm added in: reference documentation Our own reference documentation type: enhancement Major enhanvements, new features labels Jul 6, 2023
…type, @dependsOn can be used for beans of different types."

This reverts commit fba973e.
…tifact doesn't exist. org.springframework.experimental:spring-modulith-observability was the intention."

This reverts commit 1d58923.
@yossisp yossisp requested a review from odrotbohm July 7, 2023 06:00
odrotbohm pushed a commit that referenced this pull request Aug 10, 2023
@odrotbohm
Copy link
Member

That's squashed and merged, thanks!

@odrotbohm odrotbohm closed this Aug 10, 2023
@odrotbohm odrotbohm added this to the 1.0 GA milestone Aug 10, 2023
@odrotbohm odrotbohm changed the title Minor fixes in docs Polish reference documentation Aug 10, 2023
@odrotbohm odrotbohm added type: improvement Minor improvements and removed type: enhancement Major enhanvements, new features labels Aug 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: reference documentation Our own reference documentation type: improvement Minor improvements
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants