Skip to content

For #225: Implement the rest of Network methods#231

Merged
rultor merged 1 commit intoamihaiemil:masterfrom
paulodamaso:225
Dec 20, 2018
Merged

For #225: Implement the rest of Network methods#231
rultor merged 1 commit intoamihaiemil:masterfrom
paulodamaso:225

Conversation

@paulodamaso
Copy link
Contributor

For #225: Added test methods for Network.connect and Network.disconnect

- Added test methods for Network.connect and Network.disconnect
@coveralls
Copy link

Pull Request Test Coverage Report for Build 402

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 8 unchanged lines in 1 file lost coverage.
  • Overall coverage decreased (-1.2%) to 86.93%

Files with Coverage Reduction New Missed Lines %
src/main/java/com/amihaiemil/docker/RtNetwork.java 8 38.46%
Totals Coverage Status
Change from base Build 401: -1.2%
Covered Lines: 572
Relevant Lines: 658

💛 - Coveralls

1 similar comment
@coveralls
Copy link

Pull Request Test Coverage Report for Build 402

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 8 unchanged lines in 1 file lost coverage.
  • Overall coverage decreased (-1.2%) to 86.93%

Files with Coverage Reduction New Missed Lines %
src/main/java/com/amihaiemil/docker/RtNetwork.java 8 38.46%
Totals Coverage Status
Change from base Build 401: -1.2%
Covered Lines: 572
Relevant Lines: 658

💛 - Coveralls

@0crat
Copy link
Collaborator

0crat commented Dec 18, 2018

Job #231 is now in scope, role is REV

@0crat
Copy link
Collaborator

0crat commented Dec 18, 2018

This pull request #231 is assigned to @bkuzmic/z, here is why; the budget is 15 minutes, see §4; please, read §27 and when you decide to accept the changes, inform @amihaiemil/z (the architect) right in this ticket; if you decide that this PR should not be accepted ever, also inform the architect; this blog post will help you understand what is expected from a code reviewer; there will be no monetary reward for this job

@0crat
Copy link
Collaborator

0crat commented Dec 18, 2018

Job gh:amihaiemil/docker-java-api#231 already assigned to @bkuzmic, can't assign to @bkuzmic

@bkuzmic
Copy link
Contributor

bkuzmic commented Dec 18, 2018

@amihaiemil I think that puzzle number 231 is interfering with pull request 231. How to properly number the puzzle to avoid collision?

@bkuzmic
Copy link
Contributor

bkuzmic commented Dec 19, 2018

@paulodamaso @amihaiemil Adding tests and just ignoring them just feels wrong to me. I would rather create a partial implementation (or fake) that would make the test pass. This is not TDD and it doesn't give the developer immediate feedback of the correctness of the test or implementation.
If someone forgets to remove the @Ignore annotation, it would seem that everything is ok.
Regarding the dropping of the coverage - in the future I'll avoid adding dummy unsupported operation methods and just leave the puzzle to implement the interface. WDYT?

@paulodamaso
Copy link
Contributor Author

paulodamaso commented Dec 19, 2018

@bkuzmic I think that creating a partial or fake implementation that makes the test pass is worst, because it will give the impression that everythin is ok, even with "incorrect" code. I agree that leaving an @Ignore isn't the best solution, but rarely the 30min timeframe allows us to create test and code together; so Ithink that it's better to create a test method that works and it's "done" then create two fake pieces of code that will have to be refactored later.
Please note that in every puzzle left after the implementation of a test we must declare that the tests should be un-igonred, so if the developer just forgets to remove the annotation he is forgetting to resolve the task.

@bkuzmic
Copy link
Contributor

bkuzmic commented Dec 19, 2018

@paulodamaso Well, if you follow the TDD principles strictly (just to be clear, I'm not a fan of strict TDD), then it is ok to write a smallest peace of code to make a test pass and then refactor the test and then change the implementation again. Basically, this is what I've done when I created tests for all the "UnsupportedOperationException" methods.
I understand your point about the comments in the puzzle and timeframe, but I would rather force the developer to create a test that will pass (preferably by refactoring the test for UnsupportedOperationException).
Let's just conclude with the statement that I don't like comments in the source code :)

@bkuzmic
Copy link
Contributor

bkuzmic commented Dec 19, 2018

@paulodamaso Btw. do you know the answer to the question:

@amihaiemil I think that puzzle number 231 is interfering with pull request 231. How to properly number the puzzle to avoid collision?

Copy link
Contributor

@bkuzmic bkuzmic left a comment

Choose a reason for hiding this comment

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

@paulodamaso Thanks for the PR. Let's merge it.

@bkuzmic
Copy link
Contributor

bkuzmic commented Dec 19, 2018

@rultor merge it.

@rultor
Copy link
Collaborator

rultor commented Dec 19, 2018

@rultor merge it.

@bkuzmic Thanks for your request. @amihaiemil Please confirm this.

@paulodamaso
Copy link
Contributor Author

paulodamaso commented Dec 19, 2018

@bkuzmic Just for the record:

Well, if you follow the TDD principles strictly (just to be clear, I'm not a fan of strict TDD), then it is ok to write a smallest peace of code to make a test pass and then refactor the test and then change the implementation again. Basically, this is what I've done when I created tests for all the
"UnsupportedOperationException" methods.
I understand your point about the comments in the puzzle and timeframe, but I would rather force the developer to create a test that will pass (preferably by refactoring the test for UnsupportedOperationException).

Writing the test first helps us to define the requirements of our code, and we just skip the step of creating fake or mock tests for our code. Note that when we create a code that does not work and then a test just to fullfill the coverage requirements (that tests a wroing behavior) we end up with two problems, a code that does not what it was supposed to and a test that does not test the correct behavior of the code.

Let's just conclude with the statement that I don't like comments in the source code :)

Me neither! But puzzles are not comments; in 0pdd implementation we use some special tag to define them, but talking about strict XDSD we could open these issues concurrently to our tickets in our issue base; 0pdd and its syntax is just a tool to ease the job decomposition

Last, but not least, I did not understood your doubt about puzzle 231. I've never had to manually number an issue, at least that I remember

@amihaiemil
Copy link
Owner

@bkuzmic

I think that puzzle number 231 is interfering with pull request 231. How to properly number the puzzle to avoid collision?

I also don't understand your question :)
Github assigns Issue/PR numbers automatically, we simply specify the parent-issue in a new Puzzle, that's it. Or am I missing something?

@bkuzmic
Copy link
Contributor

bkuzmic commented Dec 20, 2018

@amihaiemil Ok, I got it all wrong. I thought that I needed to increment a puzzle number to next higher number. My mistake, sorry :-(.

@amihaiemil
Copy link
Owner

@rultor merge it

@rultor
Copy link
Collaborator

rultor commented Dec 20, 2018

@rultor merge it

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

@rultor rultor merged commit efc18dc into amihaiemil:master Dec 20, 2018
@rultor
Copy link
Collaborator

rultor commented Dec 20, 2018

@rultor merge it

@amihaiemil Done! FYI, the full log is here (took me 2min)

@0crat
Copy link
Collaborator

0crat commented Dec 20, 2018

Job was finished in 36 hours, bonus for fast delivery is possible (see §36)

@0crat
Copy link
Collaborator

0crat commented Dec 20, 2018

The job #231 is now out of scope

@0crat
Copy link
Collaborator

0crat commented Dec 20, 2018

Order was finished: +20 point(s) just awarded to @bkuzmic/z

@0crat
Copy link
Collaborator

0crat commented Dec 20, 2018

Payment to ARC for a closed pull request, as in §28: +10 point(s) just awarded to @amihaiemil/z

bkuzmic pushed a commit to bkuzmic/docker-java-api that referenced this pull request Jan 7, 2019
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.

6 participants