Skip to content

Conversation

@thaJeztah
Copy link
Member

Apparently I had this in my local checkout, but never committed or pushed it πŸ˜‚. Don't know where I left off, but it seems to be complete πŸ€·β€β™‚οΈ

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Copy link
Member

@crazy-max crazy-max left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@thaJeztah
Copy link
Member Author

To be fair. the "diff" (actual/expected) may be a bit wonky πŸ˜… - let me know if that's a problem

@crazy-max
Copy link
Member

Yeah a bit, do you have an issue with transitive deps on another repo?

@thaJeztah
Copy link
Member Author

Not directly, so I'm fine waiting with this. I think I originally looked at what it was used for, and all seemed "trivial enough to not need a dependency for that", but now looking again, the reason I didn't push may have been because of this specific case.

I'll give it some thinking (perhaps we can only add go-cmp (e.g.)

@crazy-max crazy-max marked this pull request as draft March 9, 2022 13:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants