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

feat: Support configurable display of version #80

Merged
merged 4 commits into from
Oct 13, 2023
Merged

Conversation

Roiocam
Copy link
Member

@Roiocam Roiocam commented Oct 13, 2023

Refs: apache/pekko-site#33

Then we can hidden page versioning on "top-level-page" of Pekko website, and still display by default for other project, all those stuff can switch via configuration.

usage

set it by specifying the flag in build.sbt

paradoxProperties += ("disabled.versioning.display" -> "true")

example

截屏2023-10-13 19 48 56

@Roiocam Roiocam changed the title chore: remove unnecessary versioning feat: Support configurable display of version Oct 13, 2023
@Roiocam Roiocam marked this pull request as draft October 13, 2023 10:17
@pjfanning
Copy link
Contributor

We use this plugin for all our docs. It is only on 1 repo where we want to remove the version (https://github.com/apache/incubator-pekko-site). Could we change this so the version display is configurable (on by default - but possible to use a config to disable it)?

@pjfanning
Copy link
Contributor

as an fyi, there has been discussion about not using paradox and this plugin for generating the top level pages in https://github.com/apache/incubator-pekko-site - switching to using another tool that is more suited for the static content that we want on these top level pages

fyi @mdedetrich

@Roiocam Roiocam marked this pull request as ready for review October 13, 2023 11:52
Copy link
Contributor

@mdedetrich mdedetrich left a comment

Choose a reason for hiding this comment

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

Am I missing something or is there supposed to be paradoxProperties += ("disabled.versioning.display" -> "true") somewhere in the code, it seems you have only committed the resulting rendered site and forgot the actual config?

@mdedetrich
Copy link
Contributor

as an fyi, there has been discussion about not using paradox and this plugin for generating the top level pages in https://github.com/apache/incubator-pekko-site - switching to using another tool that is more suited for the static content that we want on these top level pages

fyi @mdedetrich

I still have this sentiment but I am not going to block changes like this over it

@Roiocam
Copy link
Member Author

Roiocam commented Oct 13, 2023

Am I missing something or is there supposed to be paradoxProperties += ("disabled.versioning.display" -> "true") somewhere in the code, it seems you have only committed the resulting rendered site and forgot the actual config?

@mdedetrich No, you are not missing anything. This PR using paradox boolean expression which selective display the versioning via config.

$ if (!page.property_is.("disabled.versioning.display").("true")) $

No config change because it enable display by default, And it will only be hidden in explicit configuration (such as repo pekko-site need to do)

An additional pull request is required for the pekko-site repo.

Copy link
Contributor

@mdedetrich mdedetrich left a comment

Choose a reason for hiding this comment

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

nvm I was being daft, lgtm

@pjfanning
Copy link
Contributor

looks like it should work - I plan to test this on my laptop later (uptake in pekko-site repo) and can merge it if that test goes ok

@Roiocam
Copy link
Member Author

Roiocam commented Oct 13, 2023

looks like it should work - I plan to test this on my laptop later (uptake in pekko-site repo) and can merge it if that test goes ok

@pjfanning Thank you for testing. When the KV is not matching ("disabled.versioning.display" -> "true"), the hiding should also not work.

Copy link
Contributor

@pjfanning pjfanning left a comment

Choose a reason for hiding this comment

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

lgtm

@pjfanning pjfanning merged commit 1848e5a into apache:main Oct 13, 2023
1 check passed
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.

3 participants