-
Notifications
You must be signed in to change notification settings - Fork 56
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
Fix: reorganize index structure for users #188
Conversation
@sneakers-the-rat it looks like i tried to start rerouting your pr's here. is this one we should rebase and look at together? or can you reroute pr's that were on my fork to the main branch here? |
Ya ya will do shortly ♥ |
@sneakers-the-rat @lwasser What is the next step for this PR? Rebase? |
@@ -69,7 +69,7 @@ h1 { | |||
color: var(--pyos-h1-color); | |||
} | |||
h2 { | |||
margin-top: 80px; | |||
margin-top: 1em; |
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.
@sneakers-the-rat we can merge this but i have a question. i see
em, px which is fixed and rem used. and i never know when to use what approach. i'm curious what your thoughts are!! :) just a learning moment for me.
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.
Basically the way I think about it is px is rarely the "right" thing for responsive because what really matters most of the time is relative size - 1px is very different as a proportion of screen on different displays. But it is definitely the easiest and most predictable to reason about.
em and rem are both relative units, but em is relative to the current (parent) font size and REM is relative to the root size. So REM when you might use the class in multiple contexts but want the thing to look the same, em when you would want it to scale. I just default em.
With padding/margins specifically I think em is easier to think about bc I am looking at the page and can eyeball "I need about a text height space there" vs. I have no idea what that is in px. And the scale isnt necessarily linear but the spacing you need scales with the text size anyway so you need to do fewer fine tuning things over time. When you see a page where all the spacings are subtly wonky its usually because the sizes use px/other absolute unit (and also maybe not using sass in an era before easy css variables and calc)
@@ -56,7 +56,7 @@ Community docs | |||
|
|||
Publish your docs | |||
``` | |||
## _new_ Tutorial Series: How to Create a Python Package | |||
## _new_ Tutorial Series |
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.
## _new_ Tutorial Series | |
## _new_ Tutorial Series: Create a Python package |
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.
How about a middle ground. i just know that header tags matter in search results. And people skim
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.
Either way! All these are always just suggestions.
@@ -100,7 +100,7 @@ by the community now! Join our community review process or watch development of | |||
::::: | |||
|
|||
|
|||
## Python packaging ecosystem overview & best practices | |||
## Packaging |
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.
## Packaging | |
## Python Packaging for Scientists |
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.
@willingc thanks. this can be merged except i want two small changes. i have no idea what's happening - i can't approve the pr. but i'm an admin.
ok i can't approve it but i should be able to make edits to the files. i'm really confused |
Let's merge and i'll make the changes myself in a separate pr |
Works for me. Wonder what happened !? |
Thanks for doing this @lwasser and @sneakers-the-rat. That's the first that I have seen that as an admin. If it happens again, let me know. |
so odd. Here is my theory @sneakers-the-rat @willingc ... i think i actually opened this pr because Jonny had opened the pr against my other pr and i had missed them (they were in my fork). So i didn't want to lose jonny's ideas and re-opened the pr against the main branch. But the pr is still from jonny's fork. and i wonder if i didn't check "allow maintainer edits" or something? which is weird because i'm an owner of this repo but i created a PR from a fork that i do not own. i'm really not sure :) but i just made those 2 tiny edits to the headers in a separate pr so all good. the changes here as fantastic!! Thank you Jonny for submitting this and being so thoughtful about user experience of our guide! |
and @willingc thank you for helping with handling all of these pr's. i am definitely behind so this is hugely helpful to be able to pop in and review / merge or just for you to merge things that are ready to go!! |
this is a pr from @sneakers-the-rat that was sent to my fork. i'm moving it here so we can work on it once the other pr #181 is merged in a few hours!