Skip to content

Conversation

@josenavas
Copy link
Contributor

Tests are expected to fail. Note that this is going to the branch for fixing 1084.

Makes the needed DB changes.
The DB changes in the patch could be done in SQL. However, before I can execute those changes I need to perform some DB and filesystem cleaning (i.e. moving files to the upload folder again), so that's why those changes are performed in the python patch.

Copy link
Member

Choose a reason for hiding this comment

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

point -> points

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@josenavas
Copy link
Contributor Author

Thanks @antgonza! Comments addressed.

Copy link
Contributor

Choose a reason for hiding this comment

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

Just to confirm, there's no possible way we would get to this line if there are no studies right? i.e. is there any way that studies could be an empty list? If so, then min would error.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nope, there will be at least one study.

@ElDeveloper
Copy link
Contributor

👍, very minor comments.

@josenavas
Copy link
Contributor Author

Thanks @ElDeveloper, I've addressed the comments.

@ElDeveloper
Copy link
Contributor

Thanks @josenavas, 👍

On (May-25-15|15:06), josenavas wrote:

Thanks @ElDeveloper, I've addressed the comments.


Reply to this email directly or view it on GitHub:
#1201 (comment)

@josenavas
Copy link
Contributor Author

@antgonza ?

antgonza added a commit that referenced this pull request May 26, 2015
@antgonza antgonza merged commit 5489236 into qiita-spots:fix-1084 May 26, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants