Skip to content
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

Merge cmpopts and cmp package? #136

Open
zombiezen opened this issue Apr 17, 2019 · 3 comments
Open

Merge cmpopts and cmp package? #136

zombiezen opened this issue Apr 17, 2019 · 3 comments

Comments

@zombiezen
Copy link
Contributor

As a semi-frequent user of the cmp package, most of the time I pull up godoc.org reference is to figure out whether a particular useful option is in cmpopts or the cmp package. Genuinely curious: is there a meaningful end-user benefit to keeping these packages separate? I seldom use cmp without cmpopts, and I suspect this is common.

/cc @dsnet

@dsnet
Copy link
Collaborator

dsnet commented Apr 17, 2019

The original intention was to design a comparison library that was highly extensible with a small number of options.

Implementing cmpopts as a separate package kept the implementation of cmp honest such that cmpopts does not rely on internal details of cmp. In other words, cmpopts is forced to be entirely implementable in terms of the first-class options provided by cmp. This was done in reaction to other comparison libraries I looked at that supported many options and each option deeply touched internal details of the overall comparison logic.

That said, from a end-user perspective, I can see how the two being in the same package can be convenient. At this point in time, it's a bit awkward to merge cmpopts into cmp since there would now be two ways to get at the cmpopts functionality.

@neild
Copy link
Collaborator

neild commented Apr 17, 2019

At this point in time, it's a bit awkward to merge cmpopts into cmp since there would now be two ways to get at the cmpopts functionality.

Presumably everything in cmpopts would be replaced with a forward to the corresponding symbol in cmp, and marked as deprecated.

I think the separation makes a lot of sense from the perspective of the implementation, but it's pretty confusing as a user. Even as a user with an above-average understanding of cmp's internals.

@meling
Copy link

meling commented Jul 24, 2020

I came here to ask the same question as @zombiezen. I just want to point out that the cmpopts package isn't prominently visible from the cmp documentation, so as a user I spent a bit of time to find what I was looking for, and couldn't in the cmp package alone. When I eventually discovered the cmpopts package I found what I needed.

So I would also encourage merging the two packages to make the documentation easier to use.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants