Respect targets while printing dependency tree #4561
Conversation
|
Thanks for the PR! I'll review shortly. |
|
Actually, I'll rebase & force push first — I'm not confident on github's handling of review comments in the presence of force pushes, and I want to run tests on latest master. Thanks! |
dbaynard
force-pushed
the
4552-tree-deps-with-targets
branch
from
0df0d23 to
029022e
Compare
dbaynard
left a comment
dbaynard
left a comment
There was a problem hiding this comment.
I've nearly reviewed it all. I've made some changes to a number of bits (not limited to this PR; there are some uninformative variable names in related code) which I'll push tomorrow, when I'm done.
Meanwhile, there are a few questions.
Also, it would be good to have some tests in the unit test suite, not just the integration tests. I'm happy to tackle this — though it you want to, by all means go ahead.
|
Ah but we can't call it |
|
I'm finding testing to be a real pain, here. I wrote a short test suite, but because the tree is printed immediately after it has been generated it needs an io-capture like wrapper. I'm considering moving to use |
|
For now, if you're able to add an integration test that tests this codepath, I'd be able to review and merge, and we can deal with refactoring and unit tests after a merge. |
4 participants
Please include the following checklist in your PR:
Fixes #4552