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.

Sign up for GitHub

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

Jump to bottom

#561 added languages tweets #761

Merged
merged 2 commits into from
Jan 28, 2015
Merged

#561 added languages tweets #761

merged 2 commits into from
Jan 28, 2015

Conversation

krzyk
Copy link

@krzyk krzyk commented Jan 24, 2015

I had to add suppress for PMD, because it wasn't counting correctly the amount of StringBuilder reserved (PMD stated that builder was initialized with 16 capacity, while it is with 200)

@krzyk krzyk mentioned this pull request Jan 25, 2015
Closed
@alex-palevsky
Copy link
Contributor

Many thanks, I'll find someone to review it before we merge

@alex-palevsky
Copy link
Contributor

@longtimeago review this pull request when possible

longtimeago
longtimeago reviewed Jan 26, 2015
View reviewed changes
src/test/java/com/rultor/agents/twitter/TweetsTest.java
);
Mockito.when(repo.languages()).thenReturn(langs);
final Twitter twitter = Mockito.mock(Twitter.class);
final Agent agent = new Tweets(repo.github(), twitter);
Copy link

Choose a reason for hiding this comment

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

looks like one-time variable

@longtimeago
Copy link

@krzyk Please, see several remarks in the code.
Actually, actually, jcabi/jcabi-github#923 is already implemented, so we can start using MkGithub from this issue (if there is no other blocker), because mocking all github stuff looks very ugly.

@krzyk
Copy link
Author

krzyk commented Jan 26, 2015

@longtimeago jcabi/jcabi-github#923 was implemented but not all the puzzles, right now the code is in a state where MkGithub languages throws unimplemented exception

@krzyk
Copy link
Author

krzyk commented Jan 26, 2015

@longtimeago I took another look and the code is here after all, something changed during the weekend

@krzyk
Copy link
Author

krzyk commented Jan 26, 2015

@yegor256 could you release new version of jcabi-github? This way I could get rid of todo here and have full implementation ready.

@yegor256
Copy link
Owner

@krzyk jcabi-github 0.18.9 will be released in a few minutes, see jcabi/jcabi-github#972

@krzyk
Copy link
Author

krzyk commented Jan 28, 2015

@longtimeago take a look now, with newer jcabi-github it is much cleaner :)

@longtimeago
Copy link

@krzyk totally agree, thank you!

@longtimeago
Copy link

@rultor good to merge

@rultor
Copy link
Collaborator

rultor commented Jan 28, 2015

@rultor good to merge

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

@yegor256
Copy link
Owner

@rultor merge

@rultor
Copy link
Collaborator

rultor commented Jan 28, 2015

@rultor merge

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

@rultor rultor merged commit d8a0e38 into yegor256:master Jan 28, 2015
@rultor
Copy link
Collaborator

rultor commented Jan 28, 2015

@rultor merge

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

@alex-palevsky
Copy link
Contributor

@longtimeago Much obliged! I've added 16 mins to your account in payment "50957905". +16 to your rating, your total score is +1514

@alex-palevsky
Copy link
Contributor

@rultor deploy now pls

@rultor
Copy link
Collaborator

rultor commented Jan 30, 2015

@rultor deploy now pls

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

@rultor
Copy link
Collaborator

rultor commented Jan 30, 2015

@rultor deploy now pls

@alex-palevsky Done! FYI, the full log is here (took me 14min)

@krzyk krzyk deleted the 561-2 branch October 10, 2015 12:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@krzyk @alex-palevsky @longtimeago @yegor256 @rultor