Skip to content

Conversation

smacker
Copy link
Contributor

@smacker smacker commented Feb 28, 2018

Implements endpoint needed for #160

@smacker smacker requested a review from bzz February 28, 2018 16:24
@smacker smacker mentioned this pull request Mar 1, 2018
5 tasks
@bzz bzz requested a review from carlosms March 1, 2018 09:51
return results, nil
}

// Create experiment model in database
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd mention that the new ID is also set.

// Create stores an Experiment in database. On success the assigned ID is set

Copy link
Contributor

@carlosms carlosms left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@bzz bzz left a comment

Choose a reason for hiding this comment

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

lgtm

👍 for having tests

Probably a nit-pick, but having changes with refactoring in a separate commit from the changes that adds new functionality would make this PR even better.

@smacker smacker mentioned this pull request Mar 1, 2018
Signed-off-by: Maxim Sukharev <maxim@sourced.tech>
@smacker
Copy link
Contributor Author

smacker commented Mar 1, 2018

rebase, squshed, merging after CI is green

@smacker smacker merged commit 1b9b6ae into src-d:master Mar 1, 2018
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