Switch to a pip-tools workflow #67
Conversation
This is motivated by the issues around pipenv always upgrading every dependency whenever you touch any dependency. (See pypa/pipenv#2665 and pypa/pipenv#2412)
Until jazzband/pip-tools#882 is solved, this is probably as reliable as we're going to get.
| ``` | ||
|
|
||
| ### Running development server | ||
|
|
||
| ``` | ||
| pipenv run serve | ||
| mkdocs serve # or ./scripts/serve |
There was a problem hiding this comment.
As the commands are this short, I think it's ok to omit the scripts. The scripts are also not windows compatible (sans WSL), which it would be nice to maintain 1st-party support for.
Also, currently the scripts imply that the virtualenv has been activated, which isn't explicitly documented here, so we should probably document that!
There was a problem hiding this comment.
If there was tab completion on mkdocs I'd agree about omitting the scripts.
There was a problem hiding this comment.
I was actually wondering about including windows implementations for the scripts, so that if we happen to change the build setup again we could do so without needing to change what contributors need to run.
I've mixed feelings about documenting that the virtualenv needs to be active. We weren't doing do before and you don't seem to be advocating to do so for running the mkdocs commands directly. I'm therefore not sure why the scripts should be treated any differently.
In any of these cases (including the previous setup) the reader still needed to manage their environment themselves to ensure that the command being run was available. Nothing has changed in that regard.
There was a problem hiding this comment.
you don't seem to be advocating to do so for running the mkdocs commands directly
Sorry, I was definitely suggesting we do this. They were separate suggestion points.
Including windows scripts could also work, but it's a lot of extra scripting. Having the scripts auto-source the virtualenv would definitely make them more useful in my eyes.
There was a problem hiding this comment.
Having the scripts auto-source the virtualenv would definitely make them more useful in my eyes.
How do you propose that they do that? They won't know where the virtualenv is, nor even if one is needed. Setup of the virtualenv is up to the developer at the moment (this has not changed).
There was a problem hiding this comment.
This entire thread feels like we are reimplementing pipenv using an less standard tool. I'd personally rather we used a PEP 518 compliant tool, rather than yet another non-standard one.
This is motivated by the issues around pipenv always upgrading every dependency whenever you touch any dependency.
Please could you give some specific examples of where this has caused an issue with this repo? I dislike pipenv, but it is probably the best tool for this project.
There was a problem hiding this comment.
There was a problem hiding this comment.
studentrobotics.slack.com/archives/CEG05AAG6/p1567264146004600
Okay, I agree that this is an issue.
However, I still don't think that we should be working around the limitations of pip-tools by hacking virtualenv activation into scripts. If that is a required feature of the toolset, I suggest we use a tool that does it. Alternatively, we could use pip-tools and provide instructions on how to use a virtualenv / pip-tools. A link to the appropriate docs is sufficient imo.
There was a problem hiding this comment.
Currently the scripts depend on
pipenv, which handles the virtualenv stuff for the user. I think it feels fine that the new scripts also handle it.
Well, sort-of. In reality pipenv isn't going to be installed for most people by default, either in their host system or within a virtualenv they create. As a result, they end up needing to either install it in this virtualenv (which means they need to activate this virtualenv first anyway) or learn the mastery of having it in another virtualenv without that virtualenv being fully active when they run things (yet having pipenv on PATH). The former of these has pipenv just be a complication over the regular python tools (i.e: pip) and the latter requires substantial Python & system admin-fu which it seems unreasonable to expect from casual contributors.
If we want to take the opinion that this repo wants to control the virtualenv which it runs in, then we should provide scripts and documentation around creating that, rather than assuming that the user knows how to do so. Until now, it hasn't done that. (If we did do this I would argue we should still allow the user to have their environment somewhere else if they wanted, probably by having our scripts detect an existing env is active, so that people can use things like virtualenvwrapper if they want)
There was a problem hiding this comment.
This thread has gone to quite a tangent now, and isn't especially constructive to the PR.
I'd not noticed the documented setup instructions don't mention a virtualenv. I can live with the scripts staying there, and as the platform-agnostic commands are documented first, that should be fine.
I'd be interested in simply documenting that virtualenvs exist, and are considered best practice, so volunteers don't pollute their global environment.
|
#115 simplified things considerably. I do still think we're going to want something here, though this is no longer pressing. |
This is motivated by the issues around pipenv always upgrading every dependency whenever you touch any dependency.
(See pypa/pipenv#2665 and pypa/pipenv#2412)
This PR is partly to show what this would look like, as a point of comparison against the current setup.