orzubalsky / tradeschool

Trade School is an alternative school that runs on barter
Other
16 stars 5 forks source link

Some build things I hit while setting up a dev environment #145

Closed dnephin closed 10 years ago

dnephin commented 10 years ago

Hello,

I started by getting the project setup and running the tests. The first thing I hit was that I required setuptools >= 0.7 but I only had setuptools 0.6. I worked around this by using a virtualenv and installing the latest setuptools.

Have you considered using virtualenv to manage dependencies? I believe it is taking over as the standard way of creating isolated python environments. I've worked with it quite a bit and I could set it up if you're interested.

Next I ran into a missing dependency (polib), so I added that, and I guess I got a different version of django-countries because one of the imports was not working. I fixed the import and pinned the version. Another nice thing about virtualenv is that it actually provides a way to dump all the versions you are using to a requirements.txt file (using pip freeze) so you can ensure that everyone else is running with the exact same versions during development and deployment.

I moved the development setup steps to a bash script (it uses -x so it still echos everything it is doing to the user), and I automated the step of setting a new secret. Let me know how you feel about this. I like automating everything that is possible. The other change I made was to make the default dev database sqlite3. This makes it a lot easier to automate the dev setup and makes it one less external dependency.

I ran the test suite, but I see these 5 falures: http://pastebin.mozilla.org/4707567

Do you get these in your environment? I looked into the email failures, and from what I can tell the "From" is not being set here: https://github.com/orzubalsky/tradeschool/blob/master/ts/apps/tradeschool/models.py#L266

I see the reply-to header is set, but in 2a96f087 it looks like the from header is not being set anymore. Do the tests just need to be updated to reflect this change?

The other test failure ES != es I'm not sure about, but seems minor. I would still like to have a green test suite before I make any changes though.

orzubalsky commented 10 years ago

Hi Daniel,

Thanks for all of this work!

I used buildout because it worked well for me in other django projects. However, I don't have much experience using virtualenv and I'm open to using it instead. The issues with the version of setuptools started since a recent buildout upgrade and it's been a hassle. The bash script looks great!

As for the tests, these are known (not crucial) failures that I didn't get around to updating yet. Setting the from header when sending emails caused problems with the way emails are set up on production (I could use help with that if you happen to know your way around mail servers and their configurations). I appreciate you running the tests and verifying all of these details.

There are still many parts of the code that don't have tests written for them. When you start developing on top of them we/I can write any extra ones that are necessary. In the meantime, would you like me to update the tests before merging?

dnephin commented 10 years ago

I fixed the email tests in 9131fec. I'm not sure about test_branch_language (tradeschool.tests.branch.BranchTestCase). It looks like just a difference in case, but I'm not sure of the implications there. I'll leave that one for you.

I'm ok with merging with a single test failue if you are. I'll look at adding virtualenv in another branch.

As far e-mail server configuration, I do have a bit of experience with that, so I might be able to help out.