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: Improve contributing docs #5708

Merged
merged 6 commits into from
Jun 4, 2022

Conversation

juhoautio
Copy link
Contributor

@juhoautio juhoautio commented May 27, 2022

Switching to directly paste-able commands by removing the extra '$ ' prefixes.

Question about the file structure

Is it really so that CONTRIBUTING.md and docs/contributing.md (which have the same content with slightly different formatting syntax) need to be kept in sync manually? If yes, may I add comments to in these files to mention that?

Question about local dev setup

Is there a known requirement for minimum Python version for developing poetry? If yes, I can document it as well. I was trying to run poetry run pre-commit run --all-files with Python 3.7 and stumbled on this:

    ERROR: Could not find a version that satisfies the requirement flake8-type-checking==1.5.0 (from versions: 0.1.0, 0.1.1, 0.1.2, 0.1.3, 0.1.4, 0.1.5, 1.0.0, 1.0.1, 1.0.2, 1.0.3)
    ERROR: No matching distribution found for flake8-type-checking==1.5.0

I switched to Python 3.8 and also activated poetry shell – then the poetry run pre-commit run --all-files succeeded. But I'm not sure is this about Python version or something else – please advise what is the recommended setup for local dev?

Pull Request Check List

(Keeping these from the PR template in case I end up making code changes, too):

  • Added tests for changed code.
  • Updated documentation for changed code.

@Secrus
Copy link
Member

Secrus commented May 27, 2022

@juhoautio CONTRIBUTING.md and docs/contributing.md are different mostly because of Hugo templating used in docs/ version. As to comment about manual sync, if you feel the need, it's okay, but add it in CONTRIBUTING.md as the docs/ version is being rendered into the website.

You should have no problem with running Poetry code on Python 3.7, pre-commit however is slightly not in sync as you noticed. That was missed probably because the pre-commit workflow runs separately from tests. Personally, I see no reason to stay on old versions and I work on 3.10. You may add info about contributions requiring 3.8.

@Secrus Secrus added the area/docs Documentation issues/improvements label May 27, 2022
@github-actions
Copy link

github-actions bot commented May 27, 2022

Deploy preview for website ready!

✅ Preview
https://website-5rq70y4qw-python-poetry.vercel.app

Built with commit be182a4.
This pull request is being automatically deployed with vercel-action

@juhoautio
Copy link
Contributor Author

As to comment about manual sync, if you feel the need, it's okay, but add it in CONTRIBUTING.md as the docs/ version is being rendered into the website.

I would prefer a visible note in CONTRIBUTING.md and a hidden comment that is not showed in the user-facing rendered website in the docs/ version. The reason why I'd like to add those is that I spent some time looking for some kind of generator/converter because I was not expecting that two files with the ~same content are maintained manually :)

@neersighted
Copy link
Member

I'm waiting on this until python-poetry/website#37 is done as we can take advantage of the new clipboard button in this PR.

@@ -1,3 +1,5 @@
<!-- The content of this file is manually kept in sync with ../CONTRIBUTING.md. There are formatting differences because this file uses Hugo templating. -->
Copy link
Contributor Author

@juhoautio juhoautio May 30, 2022

Choose a reason for hiding this comment

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

This could be mentioned:

When you make a PR, a github action runs automatically and deploys a test website using your branch.

But it would be good to know also how to build & view the docs locally. For example adding this comment here in a "hugo template file", I'm not sure how it's going to look like when it's live before the gh action has finished deploying the changes that I pushed.

I can see that workflows/docs.yml is for example cloning another repo, the python-poetry/website. But I'm wondering if there's any lightweight way to see the files from under docs/ rendered locally.

This is what I tried:

brew install hugo
hugo -v --minify

But not sure what to do, as I got this:

Error: Unable to locate config file or config directory. Perhaps you need to create a new site.
       Run `hugo help new` for details.

Total in 1 ms

Copy link
Member

Choose a reason for hiding this comment

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

You can clone the repo, use provided README file to set it up (granted, it's not the best manual, I am working on something better) and see it locally.

Copy link
Contributor Author

@juhoautio juhoautio May 30, 2022

Choose a reason for hiding this comment

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

Thanks!

I had to run npm install before npm run dev – so that's missing from https://github.com/python-poetry/website#local-development. EDIT: I should try this again from scratch, but I don't think that npm run dev is at least needed at all..

I get this in the terminal:

rollup v2.50.5
bundles src/app.js → assets/assets/app.js...

Where can I view the docs?

Copy link
Member

Choose a reason for hiding this comment

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

It should be in the public/ folder as a pile of files -- that being said, note that the action requires someone to add the Documentation tag, and presently that has to be a member of the Poetry team. It's really meant as a way for the team to view the rendered docs without a local clone, as opposed to a way to give feedback to the PR author.

We should focus on documenting how to build the website locally for this use case.

Copy link
Contributor Author

@juhoautio juhoautio May 31, 2022

Choose a reason for hiding this comment

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

I think I figured it out. Previously I was trying poetry run python bin/website build --local ./poetry which lead into some errors, and I ignored them – I shouldn't have :) Of course I didn't have ./poetry , I have ../poetry.

So, this worked:

cd website
poetry run python bin/website build --local ../poetry
hugo server -w

Web Server is available at http://localhost:1313/ (bind address 127.0.0.1)

(btw: -w, --watch watch filesystem for changes and recreate as needed (default true))

And that answered my question about "Where can I view the docs?".

This seems to be needed to apply changes without restarting the Hugo server:

hugo server --disableFastRender

But I still need to manually run poetry run python bin/website build --local ../poetry to apply any changes, which is natural because there's a script that copies files from ../poetry/docs to content/docs/. Looks like the file contents are not changed at all so probably automatic live reloading can be achieved by symlinking, for example.

I can volunteer to document this in the contributing docs if it helps, let me know.

Copy link
Member

Choose a reason for hiding this comment

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

checkout latest update to package.json and npm scripts provided in website. you can now just run npm run dev and both npm and hugo will run. There is also Makefile providing ready to use scripts for that. I don't see need for all this to be in Poetry contributing.md. Those are separate projects.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Could it be somehow documented how to test docs changes locally when contributing to poetry? If I understood correctly the website README instructions work out of the box only for the latest docs in poetry main branch, not my local poetry dev project.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

docs/contributing.md Outdated Show resolved Hide resolved
Copy link
Member

@neersighted neersighted left a comment

Choose a reason for hiding this comment

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

Going to go ahead and merge this now and let @Secrus send a follow-up with the Clipboard button as that website PR is getting bogged down.

@neersighted neersighted merged commit 3353dfd into python-poetry:master Jun 4, 2022
@abn abn mentioned this pull request Jun 6, 2022
@juhoautio juhoautio deleted the contributing_doc branch June 7, 2022 22:32
Copy link

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 29, 2024
This pull request was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area/docs Documentation issues/improvements
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants