-
Notifications
You must be signed in to change notification settings - Fork 411
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
Improve versioning plugin #2104
Conversation
e3ad4ab
to
48bafab
Compare
48bafab
to
e572f7c
Compare
- support for single module projects - version navigator is on all pages - dropdown arrow for version navigator
372ba3d
to
7d8bdc9
Compare
7d8bdc9
to
96a6647
Compare
plugins/all-modules-page/src/main/kotlin/MultimoduleLocationProvider.kt
Outdated
Show resolved
Hide resolved
plugins/base/src/main/kotlin/templating/ReplaceVersionsCommand.kt
Outdated
Show resolved
Hide resolved
plugins/templating/src/main/kotlin/templates/ReplaceVersionCommandHandler.kt
Outdated
Show resolved
Hide resolved
context.plugin<TemplatingPlugin>().query { templateProcessingStrategy } | ||
|
||
override fun invoke() { | ||
versioningHandler.previousVersions.onEach { (_, dirs) -> copyVersion(dirs.src, dirs.dst) } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
.forEach
? or drop .toMap()
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
also: why do you need a destination in previous versions rather than resolve this destination from outputDir + version number?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I use the version source to resolve a link in VersionsNavigationCreator
. I check is there a needed file in the corresponding version source folder to avoid generating the invalid links (In the navigator such version will be strikethrough). At the copying (also creating navigator) moment the destination folder can't be used since not all version folder are copied.
plugins/versioning/src/main/kotlin/versioning/VersioningPlugin.kt
Outdated
Show resolved
Hide resolved
@@ -9,6 +9,7 @@ data class VersioningConfiguration( | |||
var olderVersions: List<File>? = defaultOlderVersions, | |||
var versionsOrdering: List<String>? = defaultVersionsOrdering, | |||
var version: String? = defaultVersion, | |||
var isOnlyOnRootPage: Boolean? = defaultIsOnlyOnRootPage |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what is only on root page?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
By default it supports the old behavior The navigator was only on root page in the old plugin.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, i know because i know how it worked in the past, but users won't have the same perspective. Maybe renderVersionsNavigationOnAllPages
? Because i am not so sure that people will know what is a RootPage
plugins/versioning/src/main/kotlin/versioning/VersionsNavigationCreator.kt
Outdated
Show resolved
Hide resolved
plugins/versioning/src/main/kotlin/versioning/VersionsNavigationCreator.kt
Outdated
Show resolved
Hide resolved
plugins/versioning/src/main/kotlin/versioning/VersionsNavigationCreator.kt
Outdated
Show resolved
Hide resolved
plugins/all-modules-page/src/main/kotlin/AllModulesPageGeneration.kt
Outdated
Show resolved
Hide resolved
@@ -9,6 +9,7 @@ data class VersioningConfiguration( | |||
var olderVersions: List<File>? = defaultOlderVersions, | |||
var versionsOrdering: List<String>? = defaultVersionsOrdering, | |||
var version: String? = defaultVersion, | |||
var isOnlyOnRootPage: Boolean? = defaultIsOnlyOnRootPage |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, i know because i know how it worked in the past, but users won't have the same perspective. Maybe renderVersionsNavigationOnAllPages
? Because i am not so sure that people will know what is a RootPage
if (isExistsFile) | ||
text(version) | ||
else | ||
strike { text(version) } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not sure if everyone will know why some versions are crossed out. Can we have some tooltip or any other form of informing documentation user that this is not present in certain version. Maybe Eugene / Mike will have a better idea for that
56ee58d
to
119dc2f
Compare
I've set |
119dc2f
to
7fd9421
Compare
Please write it in the docs |
80550ae
to
e805aed
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor comments, LGTM!
plugins/versioning/src/main/resources/dokka/not-found-version.html
Outdated
Show resolved
Hide resolved
e805aed
to
24d41b3
Compare
- support for single module projects - version navigator is on all pages - dropdown arrow for version navigator
NOTE
I removed
versioning plugin
dependency frommultimodule plugin
.