-
-
Notifications
You must be signed in to change notification settings - Fork 3.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Not clear what "raw" means in various CLI flags relating to format, or how to get those behaviors #8376
Comments
My interpretation of the sample behavior in the initial post, initially made a week ago in slack DM, and now replicated here for posterity, was:
@aschmahmann asked:
... and asked if that sounded a little nuts. I emoji-replied "no", as in, that doesn't sound nuts. (And to explain that a little more, I mean: while none of those flags are what I would've expected them to be called, the behavior overall doesn't seem logically inconsistent or holey, at least.) I continued to remark:
I offered the interpretation:
Okay. So we seem to have agreed here, at that time, last week, that we have problems around clarity of expectations and the nomenclature for what "raw" ... means. That seems to be the root issue that needs resolving before taking any other decisive action. I further offered:
I continue to not be attached to whether or not that's a good idea, or a high priority. |
Here's another transcript that seems important to hoist from being ephemeral and inaccessible in slack: @willscott had attested, in a slack:
@aschmahmann asked:
At this point, because we were in the context of still trying to merge #7976, @willscott asked:
And I think we reached the relevant conclusion of this thread when @aschmahmann replied:
And @willscott agreed:
|
Continuing that DM with @aschmahmann , I also observed:
@aschmahmann confirmed that this is a roughly correct interpretation of what those flags do. |
Okay. I believe I've transcribed all the most relevant stuff from ephemeral locations. 😓 ISTM: the next step on this is figuring out what the ipfs commands are actually supposed to do. I do not know how to propose any useful code changes until those expectations are made clear. I also do not believe anything we're looking at is an IPLD layer concern at this point. We're entirely talking about CLI flags this entire time -- and legacy ones at that. I'd frankly like to back myself away from this issue. |
As @aschmahmann and I continue to talk about it in Zoom right now, his updated statement is: It's unclear if the problem here is:
|
@warpfork the thread you've copied is largely unrelated to the issue described in the original post. Poking around a bit, the reason why the git-raw codec happily interprets bytes is these lines of code https://github.com/ipfs/go-ipld-git/blob/master/marshal.go#L21-L25 which don't really have an explanation for why we encode raw bytes as a Git blob object (although I see the general idea). On the other hand decoding expects the strict Git format https://github.com/ipfs/go-ipld-git/blob/cd342110918004a57fe7c3e6d3f9fb79f65c16ce/unmarshal.go#L12. Does lenient encoding make sense here? If so I guess we can close this issue, although I suspect strict encoding is generally what's expected from codecs. |
Ah okay. If we traced the root of our questions over there, should we make an issue in that repo? (I agree that seems at least "hmmm 🤔", but do not have an instant judgement on whether its worth taking an action on, or what that action would be. And I'm not sure who owns that choice or what the priority should be.) Then -- should we close this issue? FWIW, I think the consistency issue with the |
(I'm going to be transcribing a bunch of notes from slack and other places, mostly as quotes. The result will not be organized. You're dealing with it the same way I'm stuck dealing with it, reader. Enjoy.)
Original description: "Bug where can only get raw bytes using the git codec but not round trip it. It should not do the first or also do the second."
Some example behavior that seems confusing:
There is a long internal slack thread about this: https://protocollabs.slack.com/archives/C01MFLTLKTP/p1629207570249300 That link will require login and will also break someday. ┐_(ツ)_┌
The text was updated successfully, but these errors were encountered: