Fix panic in dagreader with raw nodes #3403
Conversation
|
Ooops, I broke tests. |
License: MIT Signed-off-by: Jakub Sztandera <[email protected]>
License: MIT Signed-off-by: Jakub Sztandera <[email protected]>
License: MIT Signed-off-by: Jakub Sztandera <[email protected]>
Kubuxu
force-pushed
the
feat/dg-reader-cleanup
branch
from
5b41fd6
to
1817d10
Compare
|
Should be fixed. Let's see CI run. |
License: MIT Signed-off-by: Jakub Sztandera <[email protected]>
There was a problem hiding this comment.
Good catch and fix @Kubuxu, what would I do without you :)
Before we merge this, lets dry it up a bit, the overlap between the old NewRNSC stuff and the bufDagReader code seems a bit off.
| "io" | ||
| ) | ||
|
|
||
| type bufDagReader struct { |
There was a problem hiding this comment.
Why don't we go ahead and replace all of the NewRNSC stuff with this now? It seems weird that they both serve roughly the same purpose.
There was a problem hiding this comment.
Yeah, the interface of DagReader supersedes RSC and bufDagReader is nop-closers on bytes.
| } | ||
| dr.linkPosition++ | ||
|
|
||
| switch nxt := nxt.(type) { |
There was a problem hiding this comment.
This should just call NewDagReader now, right?
There was a problem hiding this comment.
Not really, as cases for Dir, metadata and symlinks are a bit different.
There was a problem hiding this comment.
Handled it a bit differently.
There was a problem hiding this comment.
Are you sure? They look like the same piece of code...
License: MIT Signed-off-by: Jakub Sztandera <[email protected]>
ghost
mentioned this pull request
This doesn't resolve issue yet, I have to refactor a bit so and I want to run CI over this refactor.
Resolves #3399