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: update setting up and running in cloud #936

Merged
merged 4 commits into from
Jan 30, 2025
Merged

Conversation

vdusek
Copy link
Collaborator

@vdusek vdusek commented Jan 24, 2025

  • I updated the "Setting up" and "Running in cloud" pages to make them more beginner-friendly (hopefully) and also removed mentions of Poetry. Including Poetry was unnecessary and could be confusing. Not to mention in Python there are X other package managers that we could hypotetically mention as well. So I decided to stay only with Pip and Pipx for templates. People using a package manager other than Pip will likely know how to handle the installation on their own.
  • This was reported on Slack by @honzajavorek.

@vdusek vdusek added documentation Improvements or additions to documentation. t-tooling Issues with this label are in the ownership of the tooling team. labels Jan 24, 2025
@vdusek vdusek added this to the 106th sprint - Tooling team milestone Jan 24, 2025
@vdusek vdusek requested a review from janbuchar January 24, 2025 14:04
@vdusek vdusek self-assigned this Jan 24, 2025
@vdusek vdusek added the adhoc Ad-hoc unplanned task added during the sprint. label Jan 27, 2025
Copy link
Contributor

@honzajavorek honzajavorek left a comment

Choose a reason for hiding this comment

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

🔴 I'd like to see the comment about apify init addressed, as that's my current roadblock when working on the Python course.

🔴 I don't know if you aim to support Windows, but if so, these documents need more love and verification that all works there as intended. AFAIK the venv creation and activation won't work for sure.

🟡 Otherwise I didn't find anything serious, just various unimportant nitpicks about lower/uppercase letters (I'm sorry!) and I offered several improvements to the flow of the text. I dig removing poetry from the mix.

docs/introduction/01_setting_up.mdx Outdated Show resolved Hide resolved
docs/introduction/01_setting_up.mdx Outdated Show resolved Hide resolved
docs/introduction/01_setting_up.mdx Outdated Show resolved Hide resolved
docs/introduction/01_setting_up.mdx Outdated Show resolved Hide resolved
docs/introduction/01_setting_up.mdx Show resolved Hide resolved
docs/introduction/01_setting_up.mdx Outdated Show resolved Hide resolved
docs/introduction/09_running_in_cloud.mdx Outdated Show resolved Hide resolved
docs/quick-start/index.mdx Outdated Show resolved Hide resolved
docs/quick-start/index.mdx Outdated Show resolved Hide resolved
docs/introduction/09_running_in_cloud.mdx Show resolved Hide resolved
@vdusek vdusek requested review from Pijukatel and removed request for janbuchar January 29, 2025 14:28
@vdusek vdusek force-pushed the docs-package-managers branch from eae0214 to bc163cf Compare January 29, 2025 14:29
@vdusek
Copy link
Collaborator Author

vdusek commented Jan 29, 2025

Thanks @honzajavorek for your review.

I'd like to see the comment about apify init addressed, as that's my current roadblock when working on the Python course.

I'm not sure how much detail we should provide about the Apify CLI and apify init specifically. Crawlee projects should be Actorizable like any other Python projects. If not, only the Crawlee-specific aspects should be mentioned here, while everything related to apify init should be covered in the Apify CLI docs. Mostly to ensure future-proof relevance. At least I've extended the section and added more reference to the CLI's docs.

I don't know if you aim to support Windows, but if so, these documents need more love and verification that all works there as intended. AFAIK the venv creation and activation won't work for sure.

@Pijukatel If it's not a problem, could you please address this? It is this comment.

docs/introduction/01_setting_up.mdx Outdated Show resolved Hide resolved
docs/introduction/01_setting_up.mdx Outdated Show resolved Hide resolved
@honzajavorek
Copy link
Contributor

@vdusek That's a good point that while addressing the apify init part is important, it may be a task for a different tutorial than this one. Even in such case though, I think if you provide any kind of tutorial, the reader must be able to follow it seamlessly step-by-step, even if one of the steps is "setup your project according to this link", perhaps with "run this and if it works, you verified you can continue to another step" to make it more bulletproof. If you've done that, then I'm 👍 (I don't have time to re-read it right now, just replying to your comment.)

@vdusek vdusek requested a review from Pijukatel January 30, 2025 13:21
@vdusek vdusek merged commit 2f9dae4 into master Jan 30, 2025
23 checks passed
@vdusek vdusek deleted the docs-package-managers branch January 30, 2025 15:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
adhoc Ad-hoc unplanned task added during the sprint. documentation Improvements or additions to documentation. t-tooling Issues with this label are in the ownership of the tooling team.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants