Github user aledsage commented on the issue:
https://github.com/apache/brooklyn-server/pull/480
Excellent, thanks @ahgittin - merging now.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have
Github user ahgittin commented on the issue:
https://github.com/apache/brooklyn-server/pull/480
thanks @aledsage - fixed, if these tests pass, then good to merge
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your
Github user aledsage commented on the issue:
https://github.com/apache/brooklyn-server/pull/480
@ahgittin agreed - we should not cancel the task, so "leaking" it is right
behaviour.
The build failure is real, until we commit to switching to Java 8 - please
give your opinion
Github user ahgittin commented on the issue:
https://github.com/apache/brooklyn-server/pull/480
@aledsage minor changes addressing your comments, and fixed conflicts;
would like your confirmation that an immediate `Task` "leaking" is the right
behaviour (and longer term moving away
Github user ahgittin commented on the issue:
https://github.com/apache/brooklyn-server/pull/480
@aledsage I think I've addressed these comments, TY; but now merge
conflicts with #155 are hurting; as noted there I think we should revert that.
---
If your project is set up for it, you
Github user ahgittin commented on the issue:
https://github.com/apache/brooklyn-server/pull/480
@aledsage Thanks for thorough comments. I'll review this afternoon and
revert with whether to merge or not.
One high level comment: `Task` instances _should_ be left running,
Github user ahgittin commented on the issue:
https://github.com/apache/brooklyn-server/pull/480
All should be fixed now @aledsage
https://github.com/apache/brooklyn-server/pull/480/commits/99ccc0f6c8703e924c5fff5197bf1a0c6e39bc81
adds comments and assertions for the
Github user ahgittin commented on the issue:
https://github.com/apache/brooklyn-server/pull/480
thanks @aledsage -- have done most of them. a few more will require more
thought from me, marked them so i can find them easily.
---
If your project is set up for it, you can reply to
Github user ahgittin commented on the issue:
https://github.com/apache/brooklyn-server/pull/480
Unrelated test failures in penultimate jenkins build are discussed at
https://github.com/apache/brooklyn-server/pull/560
---
If your project is set up for it, you can reply to this email
Github user ahgittin commented on the issue:
https://github.com/apache/brooklyn-server/pull/480
Pretty sure the failures reported
[here](https://builds.apache.org/job/brooklyn-server-pull-requests/1728/) are
unrelated:
```
*
Github user ahgittin commented on the issue:
https://github.com/apache/brooklyn-server/pull/480
Re `TaskFactory` being a separate PR, that would mean massaging the tests
and other places so there is a clean state where `TF` isn't supported, and a
clean state where `TF` is supported.
Github user ahgittin commented on the issue:
https://github.com/apache/brooklyn-server/pull/480
Rebased, and once #462 is merged this should be easier to review.
Cherry-picking my "slow tests" meant it automatically left this PR which was
nice.
---
If your project is set up for
Github user aledsage commented on the issue:
https://github.com/apache/brooklyn-server/pull/480
@ahgittin This is way too hard to review until #462 is merged (which
requires rebasing and a bunch of PR comments to be addressed; as I asked there,
will you have time to address those or
13 matches
Mail list logo
