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

#412: Added additional constructors to HmRsHeader #434

Merged
merged 2 commits into from
Nov 10, 2015

Conversation

bdragan
Copy link
Contributor

@bdragan bdragan commented Nov 7, 2015

#412: Added additional constructors to HmRsHeader.

@davvd
Copy link

davvd commented Nov 7, 2015

@bdragan Thanks for the pull request, let me find a reviewer..

@davvd
Copy link

davvd commented Nov 7, 2015

@pinaf it's yours, please review

*
* @author Dragan Bozanovic ([email protected])
* @version $Id$
* @since 0.25
Copy link

Choose a reason for hiding this comment

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

@bdragan 0.27 I believe

@pinaf
Copy link

pinaf commented Nov 8, 2015

@bdragan nice. 4 comments.

@bdragan
Copy link
Contributor Author

bdragan commented Nov 8, 2015

@pinaf Thanks, please see my remarks and the new commit.

@pinaf
Copy link

pinaf commented Nov 8, 2015

@rultor merge

@pinaf
Copy link

pinaf commented Nov 8, 2015

@bdragan thanks!

@rultor
Copy link
Collaborator

rultor commented Nov 8, 2015

@rultor merge

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

@yegor256
Copy link
Owner

@rultor merge

@rultor
Copy link
Collaborator

rultor commented Nov 10, 2015

@rultor merge

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

@rultor rultor merged commit d8a8c9a into yegor256:master Nov 10, 2015
@rultor
Copy link
Collaborator

rultor commented Nov 10, 2015

@rultor merge

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

@davvd
Copy link

davvd commented Nov 13, 2015

@ypshenychka please, review this issue for QA compliance, as per par.24

@davvd
Copy link

davvd commented Nov 13, 2015

@rultor deploy pls

@rultor
Copy link
Collaborator

rultor commented Nov 13, 2015

@rultor deploy pls

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

@rultor
Copy link
Collaborator

rultor commented Nov 13, 2015

@rultor deploy pls

@davvd Done! FYI, the full log is here (took me 12min)

@ypshenychka
Copy link

@pinaf
According to our QA Rules:

The code reviewer found at least three problems in the code.
Comments were mostly about design problems, not cosmetic issues.

There were two design issues found during code review, and two cosmetic concerns.
Please confirm that you'll try to concentrate more on the overall quality of the code and reveal at least 3 major issues during code review along with complaints about minor elements of the code.

@pinaf
Copy link

pinaf commented Nov 13, 2015

@ypshenychka I confirm

@ypshenychka
Copy link

@pinaf thanks

@ypshenychka
Copy link

@davvd Quality is acceptable.

@davvd
Copy link

davvd commented Nov 13, 2015

@davvd Quality is acceptable.

@ypshenychka thanks for the review, we'll try better next time

@davvd
Copy link

davvd commented Nov 13, 2015

@pinaf paied 10 mins to @ypshenychka for QA review (payment ID is 69604927). I added 23 mins to your account, many thanks for working with the project! The completion time here was AP-1SN52002UT836622X.. you're getting extra minutes here (c=8). added +23 to your rating, now it is equal to +4143

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants