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

Fix commit unless managed #162

Merged
merged 2 commits into from Mar 2, 2017
Merged

Fix commit unless managed #162

merged 2 commits into from Mar 2, 2017

Conversation

tkdchen
Copy link
Member

@tkdchen tkdchen commented Mar 1, 2017

No description provided.

In model TestCaseTag, user is an IntegerField not a foreign key
referencing to User.

Signed-off-by: Chenxiong Qi <qcxhome@gmail.com>
commit_unless_managed is deprecated and unavailable in django 1.8. This
patch replaces commit_unless_managed with transaction.atomic in
testcases and testruns app.

Related to #148

Signed-off-by: Chenxiong Qi <qcxhome@gmail.com>
@atodorov
Copy link
Contributor

atodorov commented Mar 1, 2017

The tests look good, although we're not really testing the atomic nature of these transactions. However this is fine IMO because I'm planning to refactor these SQL statements in the near future and will update the tests if necessary. I may even drop transaction.atomic() if it turns out it is not required.

@tkdchen
Copy link
Member Author

tkdchen commented Mar 2, 2017

It is not necessary to test transaction.atomic, the atomic nature. That should be ensured to work well by Django developers.

@tkdchen tkdchen merged commit bc23c63 into Nitrate:develop Mar 2, 2017
@tkdchen tkdchen deleted the fix-commit_unless_managed branch March 2, 2017 09:44
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.

None yet

2 participants