Skip to content

adds a banner to README #205

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 4 commits into from
Nov 18, 2022
Merged

adds a banner to README #205

merged 4 commits into from
Nov 18, 2022

Conversation

rachfop
Copy link
Contributor

@rachfop rachfop commented Nov 17, 2022 •
edited
Loading

What was changed

Adds a Python banner to Readme.

Why?

Repalces the #h1 title

Checklist

  1. Closes

  2. How was this tested:

  1. Any docs updates needed?

@CLAassistant
Copy link

CLAassistant commented Nov 17, 2022 •
edited
Loading

CLA assistant check
All committers have signed the CLA.

@rachfop rachfop marked this pull request as ready for review November 17, 2022 18:17
cretz
cretz reviewed Nov 17, 2022
View reviewed changes
README.md
@@ -1,4 +1,4 @@
# Temporal Python SDK
![Temporal Python SDK](scripts/_img/banner.svg)
Copy link
Member

Choose a reason for hiding this comment

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

Can you shrink the header height down considerably? I think it should be a quarter or less of what it currently is. There's no value in the precious vertical space wasted IMO. Yes that makes the snake smaller, but I think it's worth it if it's really a "banner".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Cool, Figma had a "banner" output selection for a profile banner so that should work and make it smaller. Let me know if I need to size it down anymore.

Copy link
Member

@cretz cretz Nov 17, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

I didn't mean shrink the title text so small, I just meant the image size (sorry for being unclear). Can leave the title text the same size it was. And you may want to move the snake to the right a bit since the title may get a bit close to it.

Copy link
Member

Choose a reason for hiding this comment

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

Looks great! Will make sure CI completes and merge shortly.

@cretz cretz merged commit a7d6fc3 into main Nov 18, 2022
@cretz cretz deleted the python-banner branch November 18, 2022 18:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cretz cretz cretz approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@rachfop @CLAassistant @cretz