-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
Restructure tutorial to separate instructions based on operating system #1796
base: restructure-tutorial
Are you sure you want to change the base?
Restructure tutorial to separate instructions based on operating system #1796
Conversation
This uses the currently beta feature of using GitHub Actions to deploy GitHub Pages, rather than using a gh-pages branch. This keeps the repository size smaller.
…es-branch Switch to actions/deploy-pages
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.
You also reformat sentences to have hard line breaks at 80 chars. That makes the PR very large and will also mean all translations must update a lot. How hard would it be to undo that line break change?
So @das-g suggested the semantic line breaks in #1781 as they would make it easy to comment on one sentence. They are not hard to remove, however, I would need to know whether to keep each sentence separate and have no clear defined line break or adjust to 120 or have no clearly defined line break policy at all @ekohl. What's your take on this? |
I suggested semantic line breaks
in #1781 (comment), by which I meant "for newly added paragraphs only" and for completely new paragraphs, not moved or edited pre-existing ones. Sorry if my phrasing there wasn't clear enough to convey that. We indeed shouldn't mix paragraph reflows (whether to semantic line breaks or any other convention) into PRs that also change something else, as that will obfuscate the diffs and make the changes even harder to review than not having semantic linefeeds to begin with does.) (We could of course consider converting existing paragraphs to semantic linefeeds in separate pull requests focusing only on that. I hope that Crowdin would usually came up with the same strings when splitting the text into translatable sentences, and would thus either not require re-translation or at least offer the current translation as suggestion.) |
Hi Anna, For Hacktoberfest event there are 2 of us working through this pull request (hello https://github.com/jwhhh !). We want to confirm there aren't that many actual material changes? It appears as though mainly file are rearranged and there are substantially more lines because an character line limit is now being used. We've spent some time looking here and our strategy is that we will break it up and look at the source and also look at the published page and see if it seems OK. It is harder to check if there have been any major instructions changes but if we find any of these we will check them also. Generally we feel good so far about the update, @jwhhh just observed that perhaps some of the relative paths need to be updated and is looking to commit fixes. Another option could be to release a version 2 beta kind of thing? But overall this is great work we're hoping that we can approve soon! |
Thanks @elena. I think probably having a version 2 beta would work in this case and leave version 1 as is. Let me work on that for now. I just ran a Django Girls workshop and it was sad participants getting errors because they followed instructions not meant for their platform. Made me wish we had this available sooner rather than later. |
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.
General notes:
It looks very tied together! Awesome work @amakarudze !! Looks like great improvement!
In general, I think it works as it should, and also, would be good to open to fix as people use it :) So, we try to catch as much as we can, but to make it perfect, only with more contribs <3
In Chromebook installation I see no Python anywhere, and not in Linux installation, but I see in macOS and Windows?:
@@ -1,27 +1,47 @@ | |||
--- |
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.
Do we need to do those changes with this PR? 🤔
Otherwise, I would do them separately, just so we can make it slightly smaller 😅
* [Django installation](django_installation/README.md) | ||
* [Django Installation](django_installation/README.md) | ||
* [RunCode - Django installation](runcode/django_installation/README.md) | ||
* [ChromeBook - Django installation](chromebook/README.md#glitch-cloud-ide) |
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.
Here it points to the section, not README.MD, is it supposed to be?
{% include "/django_installation/instructions.md" %} | ||
To continue, pick your platform/operating system: | ||
* [RunCode Cloud](../runcode/django_installation/README.md) | ||
* [ChromeBook](../chromebook/README.md#glitch-cloud-ide) |
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.
Here as well, should we point to README.md only?
@elena @anaschwendler @amakarudze Just following up on this PR. It's a year old now, so is all the Cloud 9 chromebook stuff still relevant? Should I merge? |
The cloud 9 stuff will lead to merge conflicts due to #1847. (It doesn't here yet, because this PR here doesn't target branch |
@das-g Okay, so is the proposed solution to merge |
I think so, yeah. Though let's first hear from @amakarudze, whether a change in the direction of this PR is still being pursued. There's some review feedback by anaschwendler that wasn't acted or commented on for almost a year. |
The tutorial now covers installation guides for 5 platforms, namely, RunCode, ChromeBook, Linux, OS X and Windows. Reading through the installation guide while skipping instructions for 4 platforms you're not using has become tedious. It also makes it difficult to add more platforms or add new content as there are many parts of the tutorial to make edits to.
Separating these installation and deployment instructions by platform makes it easy to address gaps in documentation for each platform. It also makes it easy as it removes the need to read
NOTE: If you're not using ...
in many places. This is a big change and the first restructuring exercise. Further refactoring may be necessary to reduce repetition.This pull request addresses #1792 and attempts to shorten the tutorial by grouping OS/platform related content together so that, for example, a Windows user only reads Windows content and a OS X reads only OS X content and so on. Where there are no specific requirements necessary for ChromeBook and RunCode setups, their links have been set to point to Linux instructions.
Changes in this pull request:
chromebook
,linux
,macosx
,runcode
andwindows
have been introduced to group content by operating system/platform.Installation
,Introduction to Command Line
,Python Installation
,Python Introduction
,Your First Django Project!
andDeploy
has been separated/rearranged by operating system/platform.introduction_to_command_line
,django_installation
,django_start_project
andpython_installation
have been refactored to show content by operating system and have been removed.python_introduction/README.md
file contents have been split into two partsinstructions.md
andinstructions_part_two.md
to remove out parts that are OS specific. The general parts are now included in the README.md for each platform.deploy/README.md
SUMMARY.md
has been changed to have links grouped by OS/platform.