Adds much more line number information through the NoStackAndThen class#1423
Adds much more line number information through the NoStackAndThen class#1423
Conversation
|
Is there any test we can add to make sure we don't regress here? |
|
The tests I've managed to do so far I couldn't repo it easily in alas :/ It occurs in some of the execution tests in our test suite. But not too many others. I'll have another look later see if i can do something |
|
If we can't find any test, but you still have evidence this is better, it's fine with me to merge. I'll leave it to you. Obviously it would be better to find some kind of test since we are at a stage where changes easily cause regressions. |
|
@johnynek I have a test that requires the new code to pass here. Though its not quite the problem I set out to solve(and this does solve). It tests probably a stronger contract, if we pass through a NoStackAndThen operation we pick up some line number info, so if we toPipe elsewhere its all kept. Futures are just there to take out any stack traces we can. -- Where I mostly see line number info being lost is in executions futures where after a NoStackAndThen there is no user code on the stacktrace. |
There was a problem hiding this comment.
does the order matter here? Seems to me, each unwrapping might be a different line number. Is that not the case? Do we not want a List[Array[StackTraceElement]] and just concat all of them and try to get line numbers out of each?
There was a problem hiding this comment.
We could get a line number out of each, possibly getting several line numbers. That might be better really, but it could appear more noisey. This was just attempting to do a priority of the first thing you did to the last, but thats pretty arbitrary
There was a problem hiding this comment.
Attempts to preserve everything we know now, other than the line numbers being off a little I think in the scala compiler it looks decent
TypedPipe.from(List(1,2,3))
.map()
reports both line numbers are with the .map
|
sounds good to me for now. We can keep improving. If you're happy with it, merge away! |
Adds much more line number information through the NoStackAndThen class
2 participants
Found a bunch of cases when we would just be showing toPipe and asPipe in the descriptions. In these cases the scalding line number doesn't actually seem to be helpful at all. It seemed to be as a result of passing through the NoStackAndThen class, so thread some line data through there is the goal here.