-
Notifications
You must be signed in to change notification settings - Fork 11
Stepwise migrations, command line tool, status. #14
Conversation
Stepwise migrations, command line tool, status.
script: | ||
- cd tests | ||
- nosetests |
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.
Nose is not part of the setup.py, why not just use setuptools with test
command?
https://pythonhosted.org/setuptools/setuptools.html#test-build-package-and-run-a-unittest-suite
Ah nice with stepwise and status, thanks! 😄 |
else: | ||
model.create_table() | ||
return True | ||
def __init__(self, args): |
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.
Instead of providing an optparse
instance which is hard to use during tests you could provide a dictionary. (Ex.: args.__dict__
).
Worth mentioning that args
is not a very good name, for example pdb.set_trace()
will shadow it.
@cam-stitt I took the liberty to write some feedback, I hope you look at it in the most constructive way. I'm motivated to see this package improving since we are using it, the new version has some nice improvements but it is absolutely backwards incompatible. My experience of using import os
from arnold import main
from our_package import Model, migrations
main(
direction='up',
database=Model._meta.database,
directory=os.path.dirname(migrations.__file__),
migration_module='our_package.migrations'
) The feedback I left is related mostly to some design aspects that are no longer helping a developer use arnold the same way it worked before. Please let me know if I can help with anything else. Thanks in advance for all the work on |
I appreciate the feedback. It's the only way the project can get better. I was well aware of the backward incompatibility of the update, and probably should have spent more time trying to make it work. The good news is that the update does still have a configuration file that you can setup. Maybe I need to look at how I can have the folder/migrations all setup within the config. |
Fixes #11, #12, #13.