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 #388 #2024

Merged
merged 2 commits into from
Dec 15, 2016
Merged

fix #388 #2024

merged 2 commits into from
Dec 15, 2016

Conversation

antgonza
Copy link
Member

Decided to add _check_duplicated_columns cause it avoids duplication of code, which is an internal method in the prep_template.py.

@antgonza
Copy link
Member Author

Forgot to mention that I also have code to deal with the current duplicated fields, following Gail's suggestions, so the patch shouldn't do anything in the real system. That code is being tested in the test environment and applied before deploying this patch.

@josenavas
Copy link
Contributor

Tests are failing. Code looks good. It looks like all the python patch is dealing with SQL only - is it possible to write that code in pure SQL? If it is going to be a pain - fine as it is.

@antgonza
Copy link
Member Author

Restarted tests, it was the random postgres error. I would prefer to leave the patch as is.

@ElDeveloper
Copy link
Member

👍

@josenavas
Copy link
Contributor

@antgonza it seems that there is a real error

@antgonza
Copy link
Member Author

Yes, the error was that one of our tests was adding a duplicated column 😄

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.02%) to 91.439% when pulling a1ee51a on antgonza:fix-388 into 59ba775 on biocore:master.

@ElDeveloper ElDeveloper merged commit c5c38a6 into qiita-spots:master Dec 15, 2016
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

4 participants