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

Update local execution instructions. #119

Merged
merged 1 commit into from
Jun 2, 2017

Conversation

lognaturel
Copy link
Contributor

Mostly from @dorey here. The key thing is to replace the explicit install of xlrd with a run of setup.py. Additionally, I think including the virtualenv strategy explicitly is very helpful. Suggestions appreciated!

Copy link
Contributor

@dorey dorey left a comment

Choose a reason for hiding this comment

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

Looks good.

@ukanga ukanga merged commit 1a6e5e2 into XLSForm:master Jun 2, 2017
@alxndrsn
Copy link
Contributor

alxndrsn commented Jun 2, 2017

A couple of comments on this, although I have no experience with Python:

  1. these new instructions look helpful for people who are planning to make changes to pyxform, but for general use it seems much cleaner just to directly pip install (without the -e), and not worry about virtualenv. If there are reasons why that's a bad idea I'd be really interested to learn more. Otherwise, I think it would be good to move the simple, general-user setup instructions to the top of the doc.
  2. on Add setup step to readme.md? #99 (comment), @ukanga linked https://stackoverflow.com/questions/19048732/python-setup-py-develop-vs-install, which suggests that pip install should be preferred over invoking setup.py directly

@lognaturel
Copy link
Contributor Author

Hmm, good points, @alxndrsn. Typically I would say non-developers really shouldn't be coming to GitHub to get instructions but it seems like currently there isn't really anywhere else to get them from, probably because most end-users use pyxform through various wrappers. That said, there is category of people (me prior to yesterday) who want the command-line tool but don't need to modify it. So I agree with putting those instructions at the top.

@alxndrsn
Copy link
Contributor

alxndrsn commented Jun 6, 2017

That said, there is category of people (me prior to yesterday) who want the command-line tool but don't need to modify it.

This is the category I'd choose to be in too 😄

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.

4 participants