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

E1642 Refactor review_response_map.rb #797

Merged
merged 29 commits into from Nov 16, 2016

Conversation

Projects
None yet
6 participants
@SmokingSadhus
Copy link
Contributor

SmokingSadhus commented Oct 29, 2016

No description provided.

VivekBhat and others added some commits Oct 25, 2016

@coveralls

This comment has been minimized.

Copy link

coveralls commented Oct 29, 2016

Coverage Status

Coverage increased (+0.7%) to 43.205% when pulling f1be3e0 on SmokingSadhus:master into 4ee1f91 on expertiza:master.

@yangsong8

This comment has been minimized.

Copy link
Contributor

yangsong8 commented Nov 14, 2016

I have reviewed your pull request:

  • Some of the refactoring are wrong. See the in-line comments.
  • Testing is limited. If you wanna test "final_versions_from_reviewer", you should created multiple versions of reviews first.
  • The newly-added factory code should include all attributes of certain table, instead of only including servel attributes currently needed. Unit tests should avoid create key word, and try to use build or double instead.

@yangsong8 yangsong8 closed this Nov 14, 2016

@yangsong8 yangsong8 reopened this Nov 16, 2016

@yangsong8 yangsong8 merged commit f1be3e0 into expertiza:master Nov 16, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.