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

Make incubator notice optional #116

Merged
merged 1 commit into from
Feb 26, 2024

Conversation

mdedetrich
Copy link
Contributor

@mdedetrich mdedetrich commented Feb 26, 2024

Resolves: #115

Trying this with pekko core, this is what it looks like with this change (there should be no difference in rendered text)

image

And when setting pekkoParadoxIncubatorNotice to None

image

}
import autoImport._

val version = ParadoxPlugin.readProperty("pekko-paradox.properties", "pekko.paradox.version")

val incubatorNoticeText =
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 put the text into a publicly accessible val so that if we set the default value of pekkoParadoxIncubatorNotice to None (which we will do in the future at some point) if for some reason someone wants to access the original text they can

<p>
$page.properties.("incubator.notice")$
</p>
$ endif $
Copy link
Contributor

Choose a reason for hiding this comment

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

could we include this in the if?

     <div class="logo">
        <a href="https://incubator.apache.org/" one-link-mark="yes">
          <img src="$page.properties.("assets.hostname")$assets/images/apache-incubator.svg" alt="Apache Incubator logo" id="incubator__logo">
        </a>
      </div>

we don't want the incubator logo after we leave the Incubator.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah right, will do

@mdedetrich mdedetrich force-pushed the make-incubator-notice-optional branch from 6fc4a56 to 112661f Compare February 26, 2024 09:38
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

@mdedetrich
Copy link
Contributor Author

This is what it looks like now which is a lot better

image

Merging

@mdedetrich mdedetrich merged commit b78d9f7 into apache:main Feb 26, 2024
5 checks passed
@mdedetrich mdedetrich deleted the make-incubator-notice-optional branch February 26, 2024 09:49
@Roiocam
Copy link
Member

Roiocam commented Feb 26, 2024

I have no problem with the implementation of this PR, Thanks @mdedetrich, but I have some suggestions for improving the final render: Can we change the layout of the footer?

I think the styles of https://fury.apache.org/ and https://answer.apache.org/ are easier to migrate to the Apache Top Level Project, which will not lead to a large area of white space, in the end, it will be looks like: https://storm.apache.org/ and https://opendal.apache.org/

@Roiocam
Copy link
Member

Roiocam commented Feb 26, 2024

This is what it looks like now which is a lot better

I also agree with this approach.

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.

make incubator part of the footer.st optional (a parameter)
3 participants