-
-
Notifications
You must be signed in to change notification settings - Fork 171
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
Minor modifications to the tutorial, mainly english aesthetics #284
Conversation
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.
Thank you so much for this. Very nice to get throughout review.
I agree that all your changes improve the text.
I just have a few comments regarding terminology when related to doit
.
Feel free to add your name to AUTHORS file. I think you deserve some credit :)
doc/tutorial_1.rst
Outdated
|
||
|
||
task execution | ||
-------------- | ||
|
||
``doit`` command line by default will execute all tasks defined in ``dodo.py`` module in the current directory. | ||
``doit`` comes with a command line tool to act upon the set of tasks defined in a specific file. The default file is ``dodo.py`` in the current directory, and the default action is to execute all found tasks. |
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 dont like the "default action"
because it might be confused with the term action
as used in doit tasks. So I would stick with the word execute
here, as it is terminology used for programs...
But I agree the paragraph can still be improved. What about:
doit
comes with a command line tool to act upon the set of tasks defined in a specific file. The default is a file named dodo.py
in the current directory. Without any arguments the command line program executes the all tasks from it.
doc/tutorial_1.rst
Outdated
|
||
Note that the parameter ``module_path`` is passed into task definition of ``actions``. | ||
Instead of just specifying a callable it takes a tuple *(callable, args, kwargs)*. | ||
``get_imports`` takes the path's module as a parameter. The value of this parameter to be used upon task execution is specified in the ``actions`` of the task definition. Generally speaking, each element of the ``actions`` array is a tuple *(callable, args, kwargs)*. |
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.
Can you still include the parameter name, like:
get_imports
takes the path's module as a parameter (module_path
).
@@ -596,10 +600,10 @@ Sub-tasks (items of task group) by default are not reported on ``list`` command. | |||
imports:requests.utils | |||
|
|||
|
|||
Note the task name is composed by the (base name) group task name |
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 I have not introduced it here, but a doit
task might have an attribute called basename
.
So I would like to keep consistent with doit
terminology.
I suggest we use your alteration but still mentions "basename", something like:
Note the task name is composed by the task's group name (also known as basename
).
Also interested to get feedback about the tutorial itself (as being an introduction to the tool). |
Hey man! Thanks a lot for the nice feedback, it's really encouraging. Just wanted you to know I'll work on the changes you proposed, just give me some time. |
I think the tutorial is well intended, but is still missing some love. On occasions it sounds like a list of steps, too recipe-like. I would try to give it a more personal touch, like if you were talking to the reader (this is, of course, just a matter of personal taste). I don't know if you considered it or not, but perhaps it would be nice to pour in some comparisons with A bit more motivation at the beginning would be appropriate too. Not regarding why you'd like to make an imports' dependency graph, but why would you do it using That's all I can think of at the moment. If you agree on some of these, I volunteer to start making those changes. Just let me know. Cheers. |
Agree, my writing style is very boring 😬
I have considered this. The problem is that it adds noise to people who do not know So in general I am bit sceptic that this would be an improvement to write much in comparison to make. I think 2 or 3 notes regarding its differences might be ok.
I try to not repeat much content, specially introduction, I am afraid to bore people who actually read the stuff... I mean I get annoyed when some docs keeps claiming the same thing but doesnt go into the HOW. Actually when I wrote the tutorial I pretty much plan it based on the list of use-cases.
Thanks so much. In general I agree with the changes but see a few concerns above. Note that the tutorial is kind of new compared to the main documentation, the use-case section was not written by me. Of course improvements in any area are welcome... cheers |
Lots are trivial things, but even though English is not my native language, I humbly beleive it's a bit better now.