-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Build Instructions: Fix to work with the latest Makefile #2485
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.
Likewise, I only checked the non-Windows bit, and looks good.
One suggestion, as there's no output shown, let's remove the $
and use shell
instead of console
:
- .. code-block:: console
+ .. code-block:: shell
- $ make venv
+ make venv
- .. code-block:: console
+ .. code-block:: shell
- $ make render
+ make render
This makes it easier to copy and paste the command into the terminal.
Edit: and let's do the same for the other (venv) $
bits on this page.
Done, for commands that can be run outside of a venv safely.
The |
I like the changes (since I also prefer |
|
Not in practice. Here's what happens:
This is because So I see a few paths forward: (1) Alter Comments? |
It expects to be run inside some environment that has the dependencies in the I'm always been a big fan of always using some form of isolation, and typically include at least an admonition and brief instructions on how to create one in the readmes/contributing guide/etc. of the projects I help maintain. However, especially on a repo whose audience is generally experienced Python developers who understand the basics of dependency management, where on most platforms with As such, rather than (1) massively overcomplicating |
...when dealing with code that can run outside of a venv safely.
4627cc0
to
13cedbd
Compare
...for all remaining blocks, including those that use build.py
DId rebase branch to tip of main. Did highlight that |
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.
Checked it over and LGTM, thanks @davidfstr .
@hugovk ?
Please note that I did not test the Windows-specific instructions I added, since I do not have quick access to a Windows box.