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

Streamline local and ci environments #32

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

dmlemos
Copy link
Contributor

@dmlemos dmlemos commented Oct 15, 2019

Adding a preliminary Makefile to make it easier to develop locally
Add README with couple instructions

It helped me to stay sane :)

@PhilipTrauner
Copy link
Owner

I love Makefiles, but this just assumes too many things about the users setup and preferences for my taste:

  1. It only works if pip points to a Python 3 pip installation
    If poetry is installed as a Python 2 package, the user would have to explicitly configure the Python version that is used for the virtual environment creation (poetry env use 3.7)
  2. It assumes that the user will want to install the same poetry version as the one we use in the continuous integration setup.
  3. It installs pre-commit natively, even though it is declared as a development dependency and could therefor be executed in poetrys virtual environment
  4. The install target would have to invoke cmus-osx in the virtual environment created by poetry (poetry run cmus-osx install --force)
  5. The build dependency for the install target is redundant

I'll extend the README with development setup instructions instead.

@dmlemos
Copy link
Contributor Author

dmlemos commented Oct 20, 2019

  1. It can easily be modified to check for pip3.
  2. How important is the version? I use the latest, but figured out you'd want an enforcement.
  3. This makefile is not to be used for user installation, for that there is pip.
  4. Didn't took care of this, because sometimes it is good to have the option to install using the global/system python. It only takes a command to jump right into a pipenv (which is what I am using)
  5. 😇

Feel free to add instructions and close the PR. Just wanted to make life easier for future contributions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants