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

Issue #56 #61

Merged
merged 9 commits into from Oct 7, 2015
Merged

Issue #56 #61

merged 9 commits into from Oct 7, 2015

Conversation

tmjee
Copy link

@tmjee tmjee commented Oct 3, 2015

@dmarkov
Copy link

dmarkov commented Oct 5, 2015

@tmjee Thanks, I will find someone to review this PR soon

@dmarkov
Copy link

dmarkov commented Oct 5, 2015

@dmzaytsev it's yours, please review

@@ -91,8 +91,10 @@ public AttributeUpdates(
* @param value The value
* @return AttributeUpdates
*/
@NotNull(message = "AttributeUpdate cannot be null")

Choose a reason for hiding this comment

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

@tmjee AttributeUpdates cannot be null, right?

@dmzaytsev
Copy link

@tmjee a few minor comments above

@dmzaytsev
Copy link

@tmjee todo task will be created automatically

fixed issue #56
integration test failed due to @NotNull constraint added see #60

@tmjee
Copy link
Author

tmjee commented Oct 5, 2015

@dmzaytsev done

@dmzaytsev
Copy link

@tmjee thanks

@dmzaytsev
Copy link

@rultor merge

@rultor
Copy link
Contributor

rultor commented Oct 5, 2015

@rultor merge

@dmzaytsev Thanks for your request. @yegor256 Please confirm this.

@yegor256
Copy link
Member

yegor256 commented Oct 5, 2015

@tmjee you're suppressing so many PMD checks... it's a bad idea. Wouldn't it better to make these string literals slightly different and that's it?

@tmjee
Copy link
Author

tmjee commented Oct 6, 2015

@yegor256 done

@yegor256
Copy link
Member

yegor256 commented Oct 6, 2015

@tmjee I still see this: @SuppressWarnings("PMD.AvoidDuplicateLiterals"). Let's make sure all texts are unique

@tmjee
Copy link
Author

tmjee commented Oct 7, 2015

@yegor256 done

@yegor256
Copy link
Member

yegor256 commented Oct 7, 2015

@rultor try to merge

@rultor
Copy link
Contributor

rultor commented Oct 7, 2015

@rultor try to merge

@yegor256 OK, I'll try to merge now. You can check the progress of the merge here

@rultor rultor merged commit ac1e8d1 into jcabi:master Oct 7, 2015
@rultor
Copy link
Contributor

rultor commented Oct 7, 2015

@rultor try to merge

@yegor256 Done! FYI, the full log is here (took me 6min)

@dmarkov dmarkov added CR labels Oct 8, 2015
@dmarkov
Copy link

dmarkov commented Oct 9, 2015

@dmzaytsev paid, thanks, added 19 mins to your account, payment 66929873, 73 hours and 5 mins was spent; you're getting extra minutes here (c=4); added +19 to your rating, now it is equal to +1354

@dmarkov
Copy link

dmarkov commented Oct 9, 2015

@rultor please deploy

@rultor
Copy link
Contributor

rultor commented Oct 9, 2015

@rultor please deploy

@dmarkov OK, I'll try to deploy now. You can check the progress here

@rultor
Copy link
Contributor

rultor commented Oct 9, 2015

@rultor please deploy

@dmarkov Done! FYI, the full log is here (took me 5min)

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.

None yet

6 participants