Skip to content

Conversation

llorllale
Copy link
Contributor

This PR:

@0crat
Copy link
Collaborator

0crat commented Apr 3, 2018

Job #75 is now in scope, role is REV

@coveralls
Copy link

Pull Request Test Coverage Report for Build 117

  • 15 of 15 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+2.008%) to 76.44%

Totals Coverage Status
Change from base Build 116: 2.008%
Covered Lines: 146
Relevant Lines: 191

💛 - Coveralls

1 similar comment
@coveralls
Copy link

Pull Request Test Coverage Report for Build 117

  • 15 of 15 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+2.008%) to 76.44%

Totals Coverage Status
Change from base Build 116: 2.008%
Covered Lines: 146
Relevant Lines: 191

💛 - Coveralls

Copy link
Owner

@amihaiemil amihaiemil left a comment

Choose a reason for hiding this comment

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

@llorllale Looks nice, just a question.

And, as a note, please break your tasks better. It seems to me this clearly took you more than 30min (including documentation reading and so on).

Even if you wanted to work that much, it's not ok for the project's image to accept work in more value than it pays. Well, it is not the case in this project, since there are no money involved, but you get the idea :D

}

@Override
public String init(final String listenAddress) throws IOException {
Copy link
Owner

Choose a reason for hiding this comment

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

@llorllale Why return String here and bellow? Isn't the response a Json object?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@amihaiemil
Copy link
Owner

@rultor try to merge

@rultor
Copy link
Collaborator

rultor commented Apr 4, 2018

@rultor try to merge

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

@llorllale
Copy link
Contributor Author

@amihaiemil

And, as a note, please break your tasks better. It seems to me this clearly took you more than 30min (including documentation reading and so on).

This surprised me because I found this one of the quickest ones I think. But fine... I will literally time myself next time.

@rultor
Copy link
Collaborator

rultor commented Apr 4, 2018

@rultor try to merge

@amihaiemil @llorllale Oops, I failed. You can see the full log here (spent 20s)

+ set -e
+ set -o pipefail
++ dirname ./run.sh
+ cd .
+ echo 14333
+ echo '1.67.2 b60448e97'
1.67.2 b60448e97
+ date
Wed Apr  4 13:20:44 CEST 2018
+ uptime
 13:20:44 up 10 days, 16:53,  0 users,  load average: 0.36, 0.10, 0.03
+ ff=default
+ image=yegor256/java8
+ rebase=false
+ head_branch=master
./run.sh: line 23: syntax error near unexpected token `('
'cid' file is absent, container wasn't started correctly

@0crat
Copy link
Collaborator

0crat commented Apr 4, 2018

The job #75 is now out of scope

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.

5 participants