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

[MNG-5771] Document all classloaders used in Maven #338

Merged
merged 2 commits into from
Nov 17, 2022

Conversation

kwin
Copy link
Member

@kwin kwin commented Nov 15, 2022

Clarify differences between Core and Build extensions

Clarify differences between Core and Build extensions
elharo
elharo previously requested changes Nov 15, 2022
Copy link
Contributor

@elharo elharo left a comment

Choose a reason for hiding this comment

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

Thanks for cleaning this up. Looking at it now, this was indeed very unclear.

content/apt/guides/mini/guide-maven-classloading.apt Outdated Show resolved Hide resolved
content/apt/guides/mini/guide-maven-classloading.apt Outdated Show resolved Hide resolved
content/apt/guides/mini/guide-maven-classloading.apt Outdated Show resolved Hide resolved
content/apt/guides/mini/guide-maven-classloading.apt Outdated Show resolved Hide resolved
content/apt/guides/mini/guide-maven-classloading.apt Outdated Show resolved Hide resolved
content/apt/guides/mini/guide-maven-classloading.apt Outdated Show resolved Hide resolved
content/apt/guides/mini/guide-maven-classloading.apt Outdated Show resolved Hide resolved
content/apt/guides/mini/guide-using-extensions.apt Outdated Show resolved Hide resolved
content/apt/guides/mini/guide-using-extensions.apt Outdated Show resolved Hide resolved
@@ -0,0 +1,46 @@
/*
Copy link
Contributor

Choose a reason for hiding this comment

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

What are these new files doing here? Maybe drawing some sort of diagram or something?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, exactly, they have this format: https://plantuml.com/class-diagram and are converted during build to SVG. Also they are referenced in modified APT documents e.g. in https://github.com/apache/maven-site/pull/338/files#diff-3a01086d28efd2b3cb838d37d2efcafa96e65044d0b09a6826b6a5ca3b54a45aR92.

@cstamas
Copy link
Member

cstamas commented Nov 15, 2022

Starter comment: could we just convert any exotic formatted document to something supported by modern tools when we touch them? Like Markdown or ADoc (depdending on complexity and requirements of documentation)?
I really dont understand why on earth we keep coming up with our own "standards" that just make our lives harder than should 😄

@kwin
Copy link
Member Author

kwin commented Nov 15, 2022

Starter comment: could we just convert any exotic formatted document to something supported by modern tools when we touch them? Like Markdown or ADoc (depdending on complexity and requirements of documentation)? I really dont understand why on earth we keep coming up with our own "standards" that just make our lives harder than should 😄

I don't think this should be included in PRs which change something else. Otherwise the diff is no longer visible. It needs to be separately done either before or after. Also I miss the automated conversion from APT to Markdown in https://github.com/apache/maven-doxia-converter....

@cstamas
Copy link
Member

cstamas commented Nov 15, 2022

Wow, yet another Maven Component that depends on plexus-container-default... 🍺

@elharo
Copy link
Contributor

elharo commented Nov 15, 2022

It might be worthwhile to write a general purpose .apt to markdown converter. However that should not be part of this PR. As to why we have these exotic special purpose things, a lot of it is is simply that this code goes back 20 years and no such de facto standard tool existed back then.

@kwin
Copy link
Member Author

kwin commented Nov 17, 2022

@cstamas Any further remarks before I go ahead and merge?

@kwin kwin merged commit abd7436 into master Nov 17, 2022
@kwin kwin deleted the feature/core-and-build-extensions branch November 17, 2022 15:20
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.

4 participants