-
Notifications
You must be signed in to change notification settings - Fork 838
website: include adopter logos on homepage #4065
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
base: master
Are you sure you want to change the base?
Conversation
Hi @Dylannni. Thanks for your PR. I'm waiting for a kubeflow member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
@andreyvelich Just realised everything from last time is ready for PR. Please continue confirming the companies so I can replace the placeholders I have right now. Also, what are your thoughts on case study page? I could make something fancier than this tomorrow if needed. |
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.
/ok-to-test
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.
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.
LGTM
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.
Thank you for this!
When should I replace the palceholder? |
@Dylannni What do you have in mind for the Case Studies section ? |
@andreyvelich We could have a case study board (logo with a short description) for each company, instead of just listing out the case studies. What do you think? |
Yeah, or maybe we can learn something from CNCF Case Study page: https://www.cncf.io/case-studies/ |
/hold We haven't got approval from the CNCF to use trademarks, or confirmation about which trademarks we can use, please continue that discussion on: Also, please rebase after the dark-mode PR to ensure it looks good in both. I want to take a look at the styles again once you rebase, so please tag me. |
@castrojo ccing you for legal discussion about what is required to include these logos on the Kubeflow homepage. But feel free to respond on the issue instead, incase we dont end up using this specific PR: |
49dc9dd
to
145885e
Compare
@thesuperzapper Rebased onto latest master branch, have a look! I noticed that the Amazon/RedHat logos look faint in dark mode as their name are in Black, any suggestion on how I could deal with it? |
LGTM |
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.
Given that @castrojo gave /lgtm for this PR, I think we can move forward.
/lgtm
/hold for others review
/cc @kubeflow/kubeflow-steering-committee @rimolive @varodrig @kubeflow/release-team @StefanoFioravanzo
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.
/lgtm
We've got For Google, @james-jwu please can you help us with the approval ? @chasecadet Who we can reach out for Microsoft approval ? |
cc @chensun for the Google approval. |
@andreyvelich as I was saying in #4065 (comment), to establish a process we should:
Notes:
|
Additionally for this PR to be mergable, there are a few things we need to fix. Changes:
|
@Dylannni Please would you help updating this PR to incorporate @thesuperzapper feedback:
Trusted By vs Adopted By, maybe Trusted By make sense looking at what others are doing:
@castrojo @kubeflow/kubeflow-steering-committee thoughts ? |
@andreyvelich @thesuperzapper Thank you for the feedback — I’ve made some changes accordingly. Apologies for any confusion — I initially thought this PR was meant to be a mock-up. |
@franciscojavierarceo can you I think you ended up "approving" in the eyes of Prow because of the old GitHub review being treated as a prow approval issue. |
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.
@Dylannni there are still other fixes to do based on previous comments (e.g. dark/light) but here are a few things.
/approve cancel |
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
@thesuperzapper I’ve made some changes accordingly, but I’m still facing one issue that I’m not sure how to resolve. Could you kindly help me? I’ve added both color logos and white logos in the HTML, and assigned them trusted-logo-light and trusted-logo-dark classes respectively, defined in /scss/_styles_project.scss. I want to switch to the white logo when dark theme is selected. I tried adding the following:
But it doesn’t seem to work as expected. Thank you very much! |
@thesuperzapper @kubeflow/kubeflow-steering-committee @Dylannni @castrojo What is blocking us to move this forward? |
@andreyvelich The main blocker is switching the color logo to a white logo for dark mode. I can only work on it this weekend because I have my final exam on Friday. |
Signed-off-by: Dylan Lin <[email protected]>
Signed-off-by: Dylan Lin <[email protected]>
…o Adopters page Signed-off-by: Dylan Lin <[email protected]>
Signed-off-by: Dylan Lin <[email protected]>
Signed-off-by: Dylan Lin <[email protected]>
Signed-off-by: Dylan Lin <[email protected]>
Signed-off-by: Mathew Wicks <[email protected]>
e5c9687
to
95729bd
Compare
@Dylannni @andreyvelich I have rebased the PR and fixed the build errors with 95729bd I have also very carefully created SVGs for each logo in both dark and light modes (which are exactly the same size). The next steps are to get confirmation from each listed company:
Then have the KSC vote to accept these specific logos on the homepage. |
Checklist:
Description of your changes:
Added - Companies Name/Logo, a case studies markdown file under /docs/about
Edited - Front page layout (_index.html)
Issue
Closes: #4036
Labels
/area website