-
-
Notifications
You must be signed in to change notification settings - Fork 32.6k
Add missing command for running-test on contributing page #19068
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
base: main
Are you sure you want to change the base?
Conversation
62664c8
to
c9a9d18
Compare
That step is mentioned a bit further up in the document:
My sense is that the intention of the tutorial is for new contributors to follow it from the beginning, so I think we can assume that the command has already been run. Are you thinking otherwise? |
I understand your perspective. I thought someone might misunderstand that the above process is not for the django itself, it is an order to use the django development version in another project. When I copied the document, I intended it to be executed as part of the process. That said, after hearing your explanation and revisiting the document, I think my concerns were a little too much. If most of people think like you, I will close the PR. Thanks for review! |
Thanks for considering my perspective! We can wait to see what someone else with more authority says. |
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 agree with Adam that repeating here the -e
install line feels redundant, but I also see the confusion that @baidoosik presents. I think we can make a small tweaks to this page so instead of running -e
again, it says something like "Ensure that you have followed and completed the previous section".
Proposal:
Before running the test suite, make sure you've followed the steps in the previous section, and then enter the Django tests/
directory...
"previous section" should be a local reference to the section before this one.
What do you think?
I don't understand the confusion. Why did you skip the previous section? I don't understand what "When I copied the document, I intended it to be executed as part of the process." means. I'd prefer not to start a precedent of adding "make sure you've followed the steps in the previous section" to tutorials. If readers believe they know enough to skip steps, I wouldn't worry about warning them. |
@timgraham I didn't skip the previous section. In the previous section, I totally understood I need to install local Django packages with editable mode when I want to use local Django package. So, when I failed to execute $ python -m pip install -r requirements/py3.txt command, I immediately noticed that the package needed to be installed. If agree with my opinion, tell me. I will update code with @nessita's feedback. |
I'm afraid I still don't understand how a reader ends up at "Running Django's test suite for the first time" while skipping part of the previous section. Isn't the purpose of the proposed clarification aimed at someone who did? |
@timgraham Let me clarify my point. Even if someone reads the previous section completely, first-time Django contributors might not connect that local Django installation is a prerequisite for running tests. While the previous section explains installing local Django in editable mode, it might not be immediately obvious to newcomers that this is required for running the test suite. |
Why would the step be in the tutorial if it's not necessary? We could move that step into this section (would that solve your concern?), however, these steps require cloning Django... which is in the previous section. So I don't understand why the install step is "special" in this respect. There is also: https://docs.djangoproject.com/en/dev/internals/contributing/writing-code/unit-tests/#quickstart |
I understand your perspective, Tim, but I also see where the author is coming from. As an experienced contributor, it may seem like the clarification is unnecessary. However, for someone new to Django, it's not immediately intuitive that Django needs to be installed with The mental split here is between "using" Django in an env (which requires In summary, I think we can add the briefest clarification similar to what's already present a few lines above with some tweaks:
|
@timgraham |
I see that dee687e may have added a distracting aside that broke up the flow of the tutorial. (Interesting in light of recent efforts to remove extra details from the other tutorials.) I have nothing more to say so feel free to proceed however you like. |
Thank you Tim for your valuable view and sharing your strong experience with fine-tuning docs. I agree that tutorials should not include distracting information but, to me, the reiteration of installing Django locally in editable mode is a reinforcement of a basic concept, not a distracting note. To explain myself, consider this analogy: when teaching someone how to ride a bike, you’d repeat key points like "keep your eyes focused on the road to help with balance" (mostly to avoid a fall, which is quite desirable in this scenario). This is useful, but if one would also explain how balance works, this definitely is distracting to the learner (and unnecessary at this stage). Similarly, in this tutorial, I agree that reiterating the need to install Django locally with @baidoosik Could you please consider a broader review of the two sections and ensure consistency, specifically when including "reminders" for the |
…up in docs. Added missing 'python -m pip install -e ..' command to test suite setup in docs.
c9a9d18
to
b96af08
Compare
@nessita I referred to dee687e as a potentially distracting aside that may not be consistent with the Diátaxis guidelines for tutorials. My thought was that without that aside, the step by step nature of the tutorial may be more obvious. In fact, if I understand correctly, Jordan said in (#19068 (comment)) that he was confused by the aside ("Instead, I proceeded with the "Creating projects with a local copy of Django" section...") and that's why he skipped the required install command. |
I partially agree with @timgraham opinion. While that section is helpful, I feel it slightly confuses the purpose of installing Local Dev Django. It’s as if the flow of thought is being disrupted, so I’ve been considering whether it might be better to separate that section(style), as shown in the image below. |
@nessita @timgraham I updated code. If you have any feedback, feel free to tell me. I want to make the document as accurate and easy to understand as possible. |
@nessita gentle reminder 🙏 |
This is in my review queue but there are other Fellow tasks that have more priority, sorry. I will get to this as soon as possible (but it could take weeks). |
@nessita Are you still busy? |
If I was to make changes I would:
--- a/docs/intro/contributing.txt
+++ b/docs/intro/contributing.txt
@@ -177,7 +177,7 @@ Go ahead and install the previously cloned copy of Django:
The installed version of Django is now pointing at your local copy by installing
in editable mode. You will immediately see any changes you make to it, which is
-of great help when writing your first contribution.
+of great help when testing your first contribution. |
Trac ticket number
N/A
Branch description
The command for installing the Django package was missing in the documentation. This omission may cause confusion for people following the guide, potentially leading to issues when running tests. I encountered this issue myself and found the correct command in the
tests/README.rst
.This PR adds the missing python -m pip install -e .. command to ensure that the test dependencies are properly installed before running the test suite.
Checklist
main
branch.