-
-
Notifications
You must be signed in to change notification settings - Fork 782
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
Contrib guide: more restructuring #1461
base: main
Are you sure you want to change the base?
Conversation
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 @nedbat!
Looks great. Two minor comments but don't hold merge on them:
- Perhaps center or move the links into the titles of the landing page table
- On the code contributions communications page, flip flop Discourse to come before mailing lists.
🎉
@@ -1,3 +1,5 @@ | |||
.. _c_code: |
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.
This made me think of C the language.
Are these c_
prefixes short for "contrib" and temporary whilst still a draft in parallel with the old devguide?
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.
Yes, exactly. I maybe should have explained that.
contrib/index.rst
Outdated
}); | ||
</script> | ||
<style> | ||
table.docutils td { vertical-align: top; } |
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.
Perhaps we should move this to _static/devguide_overrides.css
?
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.
I didn't know about that file. I think tables should default to top-aligned, but idk if this will affect other things badly as a global setting.
*[I haven't adjusted the links in the table yet other than to add a link to the | ||
major section at the top of each column.]* | ||
|
||
.. list-table:: |
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.
I'm trying to think which is better, lists inside a table, or smaller cells, from an accessibility point of view, and I'm not sure :)
The new table of lists might be better, because there's no relation between the old cells that happen to be next to each other; that is, the old way was just for presentation and not structure. So I think the list might be better?
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.
Other people I've talked with definitely felt like the one-link-per-cell style of the current home page table is misleading because the rows have no meaning.
contrib/index.rst
Outdated
[The contents had been listed below here. It was very long, and I don't think | ||
it was helpful. The left sidebar has the contents.] |
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.
Hmm, I found it helpful: I often cmd+F on the homepage to find something, which won't work with a collapsed left sidebar.
It is a little unusual to have a large table-of-contents on a page, although it is out of the way at the bottom of the page, and not so big as to be a performance problem (like, we definitely wouldn't want the whole CPython TOC).
But it's not the end of the world to remove it if no one else wants it :)
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.
I think it is fine to leave at the very bottom.
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.
I can leave it.
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.
I put it back, but not as deep as before, so it's not so long.
@willingc the list has Repos, Discourse, Discord, Mailing lists, so I'm not sure what you are looking for. |
@willingc I also don't understand this:
|
4939b1e
to
d771c82
Compare
I'm continuing to look at the overall structure and move things around. Comments welcome. I wanted to flesh out the Code and Docs landing pages before making a real pull request, but maybe it's ok to merge this?
📚 Documentation preview 📚: https://cpython-devguide--1461.org.readthedocs.build/