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

[maybe] Convert make to python using pyinvoke #9

Closed
wants to merge 15 commits into from

Conversation

atemate
Copy link
Contributor

@atemate atemate commented Sep 9, 2019

Possible alternative to #6.

In #6 (comment), @asvetlov proposed to use pyinvoke instead of Makefile. I've spent some time on researching this tool and here's the results:

The project seems to be alive with 2.5k stars and ~60 PRs, but many of them are very old (half a year, for example).

Pros:

  • (by @asvetlov) is documented and well-supported, compared to a home-made solution
  • (by @asvetlov) out-of-the-box supports command parameters and options
  • (by @asvetlov) supports code completion out-of-the-box (sic! source <(invoke --print-completion-script=bash))
  • no need to invent a wheel and manually implement runners and task collectors as we did Convert make to python #6

Cons:

@astaff astaff added the platform label Sep 9, 2019
@atemate atemate mentioned this pull request Sep 9, 2019
@asvetlov
Copy link

asvetlov commented Sep 9, 2019

Cons:

  • you have to pip install invoke before usage

In real life, you quickly start wanting to use additional libraries anyway. Even pip is not a part of stdlib. Simple bootstrap.sh can solve the problem.

  • a bit more complicated code of tasks (targets): required argument context, required decorator @task

I consider this as a benefit, not an objection :)

  • no colored output! so looks like a mess. Of course, we can disable printing to stdin and stdout (or redirect them to log files), but IMO we should not mute anything in order to do this tool as much transparent as possible

I cannot buy it.
invoke doesn't print anything itself. If you want to use ANSI sequences for output colorizing -- invoke doesn't strip them.

The warning is not shown by default. Would be nice to fix it eventually but it is not a show stopper.

Not sure if annotations matter in tasks.py at all. Most likely people will ignore them here anyway.

from setuptools import find_packages, setup


setup(
Copy link

Choose a reason for hiding this comment

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

What is the purpose of this setup.py file?
Do you want to publish it on pypi, import by python code, support versioning etc.?
From my perspective, tasks.py should live in the root of generated template code.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

true, we can drop setup.py in both PRs (this and #6)

@@ -0,0 +1,4 @@
debug: false
Copy link

Choose a reason for hiding this comment

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

What is the purpose of the file?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

local invoke configuration that. For example, setting echo: true prints the executed command in bold, which is very convenient. Since so far it's the only setting we actually use there, we can remove the setting file.

@atemate
Copy link
Contributor Author

atemate commented Sep 9, 2019

In real life, you quickly start wanting to use additional libraries anyway. Even pip is not a part of stdlib. Simple bootstrap.sh can solve the problem.

yes bootstrap.sh that does not use pip sound reasonable.

(no colored output) I cannot buy it. invoke doesn't print anything itself. If you want to use ANSI sequences for output colorizing -- invoke doesn't strip them.

still not working on my machine

The warning is not shown by default. Would be nice to fix it eventually but it is not a show stopper.

well, I don't know why but they are.

Not sure if annotations matter in tasks.py at all. Most likely people will ignore them here anyway.

agree.

@atemate
Copy link
Contributor Author

atemate commented Sep 9, 2019

Closing as for now we decided to stick with Makefile

@atemate atemate closed this Sep 9, 2019
@atemate atemate deleted the ay/make-to-invoke branch December 19, 2019 16:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants