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

Docs visual improvement and add documentation guide for new contributor for docs #4554

Draft
wants to merge 17 commits into
base: master
Choose a base branch
from

Conversation

gunungpw
Copy link

@gunungpw gunungpw commented Jan 3, 2022

Detail in issue:

Resolve #4553

this pull request is only for the visual change for docs and update dependency for now
for documentation guide for new contributor, still in progress.
Hope i can help for the new cython 3.0 documentation initiatives,

Link for:

Official Docs
Official Docs theme don't implement mobile view, new sphinx_rtd_theme in testing have good mobile view

Testing Docs

Sorry if my English is not too good, its not my first language

Best Regard,

Gunung P. W.

@gunungpw gunungpw mentioned this pull request Jan 3, 2022
29 tasks
@gunungpw gunungpw changed the title [ECH] Visual improvement and add documentation guide for new contributor Docs visual improvement and add documentation guide for new contributor for docs Jan 3, 2022
Copy link
Contributor

@scoder scoder left a comment

Choose a reason for hiding this comment

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

I didn't look through all changes, because some seem to lack a motivation. Could you try to explain a bit more what your goals are and why you made certain changes, especially the configuration changes in conf.py? I see a lot of options and code being commented out there now, without an explanation.

Also, all those whitespace and quoting changes make it unnecessarily difficult to see what options were actually changed.

Comment on lines +26 to +27
:doc:`src/reference/index`
How to use Cython to speedup your project.
Copy link
Contributor

Choose a reason for hiding this comment

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

We are no longer using the reference. It is only left here to keep old links alive.

Copy link
Author

Choose a reason for hiding this comment

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

Thank for info, I will comment out that from :toctree:: and add :orphan: to suppress warning in build time

------

:doc:`src/userguide/index`
Learn to use Cython package.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Learn to use Cython package.
Learn to use Cython.

Copy link
Author

Choose a reason for hiding this comment

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

This was a placeholder that I add to give context for src/userguide/index link, if the suggestion good I will give it fix.

-----------

:doc:`src/quickstart/index`
How to install Cython compiler package.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
How to install Cython compiler package.
How to install the Cython compiler package.

Copy link
Author

Choose a reason for hiding this comment

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

Thank you 👍 for the fix

How to install Cython compiler package.

:doc:`src/tutorial/index`
How to use Cython to speedup your project.
Copy link
Contributor

Choose a reason for hiding this comment

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

You are using the verb here, not the noun.

Suggested change
How to use Cython to speedup your project.
How to use Cython to speed up your project.

Copy link
Author

@gunungpw gunungpw Jan 3, 2022

Choose a reason for hiding this comment

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

Thank you again, i'm learning new thing

How to contribute changes to Cython.

:doc:`Changelog <src/changes>`
Release note of Cython development.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Release note of Cython development.
Release notes for Cython's development.

Copy link
Author

Choose a reason for hiding this comment

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

🛠️ fix

@@ -1,23 +0,0 @@
Welcome to Cython's documentation.
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do you think that this file is no longer useful?

Copy link
Author

@gunungpw gunungpw Jan 3, 2022

Choose a reason for hiding this comment

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

When I remove it, I have a plan to replace it with README.rst that has a new guide for newcomers to edit and build docs locally and link it to docs. But I realize the file has historical value to the project and is already CONTRIBUTING.rst in place for contributing guide. also, my habit to organize project directory give contributing factor. I will revert it back Thanks :)

Comment on lines +46 to +48
# try: import rst2pdf
# except ImportError: pass
# else: extensions.append('rst2pdf.pdfbuilder')
Copy link
Contributor

Choose a reason for hiding this comment

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

Why did you remove this part?

Copy link
Author

Choose a reason for hiding this comment

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

readthedocs have config for build pdf and epub in .readthedocs.yaml, and i have errors when try to build this locally. I will revert it back if I can't find ways to fix it, or many contributors need to build pdf locally

Comment on lines -64 to +65
copyright = '%s, %s' % (YEAR, authors)
project = "Cython"
authors = "Stefan Behnel, Robert Bradshaw, Dag Sverre Seljebotn, Greg Ewing, William Stein, Gabriel Gellner, et al."
copyright = f"{YEAR, authors}"
Copy link
Contributor

Choose a reason for hiding this comment

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

This is not the same.

Copy link
Author

Choose a reason for hiding this comment

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

Thanks, still learning Python

Copy link
Contributor

Choose a reason for hiding this comment

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

You're close though; f"{YEAR}, {authors}"

# except:
# pass
# # The short X.Y version.
# version = re.sub('^([0-9]+[.][0-9]+).*', '\g<1>', release)
Copy link
Contributor

Choose a reason for hiding this comment

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

Why did you remove this part?

Copy link
Author

Choose a reason for hiding this comment

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

This is my bad, when build locally I got errors so try to comment block that I don't understand

@gunungpw
Copy link
Author

gunungpw commented Jan 3, 2022

Thanks for review.

This PR is just my first try to see if the project docs need update.
For a summary, the change in conf.py is not intended to be permanent, I just want to experiment to see how new landing page in the new theme. for change in .rst file also just for the look in landing page because I still learning reST in linking between page, many part that i add is just for placeholder. Sorry for grammar mistake, i still learning English, hope you can understand. each problem i will address it at a later time. Hope you have a nice day 😄

@gunungpw gunungpw marked this pull request as draft January 4, 2022 06:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[ENH] Facelift and add guide to contribute to Cython Documentation
3 participants