Skip to content

Conversation

@josenavas
Copy link
Contributor

In the previous implementation, the addition of the prep info and the first artifact was forced to occur at the same point. We used that strategy at the beginning of Qiita and we changed it later because that did not follow the workflow.

At the same time, if the artifact creation fail for whatever reason, the entire page was broke and the user was not able to see the study page again.

Here, the addition of the artifact is removed from the addition of the prep information.
At this point, artifacts can't be added, but that will be added in the next PR, to keep them small.
Thus, the interface is broken at this point BUT the tests should pass.

Returns
-------
(list of str, list of str, dict of {str: list of str})
The list of txt,csv files in the upload dir for the given study
Copy link
Member

Choose a reason for hiding this comment

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

csv is not in the list below.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, same comment, plus txt,csv -> txt, csv

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think this is actually a typo in the documentation. We are supporting tsv (Tab Separated Values) rather than csv (Comma Separated Values).

@antgonza
Copy link
Member

antgonza commented Mar 9, 2016

One minor comment, then 👍 @squirrelo or @ElDeveloper could you review?



def new_prep_template_get_req(study_id):
"""
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you add a description to this function?

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

@ElDeveloper
Copy link
Contributor

👍 Just a few comments, looking good overall!

@josenavas
Copy link
Contributor Author

Comments addressed

@ElDeveloper
Copy link
Contributor

👍

antgonza added a commit that referenced this pull request Mar 10, 2016
…-creation

Refactoring the refactor (I): Fix prep info creation
@antgonza antgonza merged commit e886f31 into qiita-spots:artifact-study-pages Mar 10, 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.

3 participants