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

Add the symmetric difference function, so given a number of sets, I c… #21956

Closed
wants to merge 1 commit into from
Closed

Conversation

jjshoe
Copy link

@jjshoe jjshoe commented Jul 2, 2019

…an get back only unqiue items

Solves for #16044

@hashicorp-cla
Copy link

hashicorp-cla commented Jul 2, 2019

CLA assistant check

Thank you for your submission! We require that all contributors sign our Contributor License Agreement ("CLA") before we can accept the contribution. Read and sign the agreement

Learn more about why HashiCorp requires a CLA and what the CLA includes

Have you signed the CLA already but the status is still pending? Recheck it.

@ghost ghost added the sdkv1 [PRs only] Marks changes that may potentially need to be ported to the plugi nSDK label Nov 12, 2019
This computes the symmetric difference of two or more sets. This function
is provided in the cty stdlib package, so this is just a reference to that
existing implementation.
@apparentlymart
Copy link
Member

Hi @jjshoe! Thanks for working on this and sorry for the slow response.

I've pushed a new version of your change after resolving the conflicts that appeared with master in the meantime. I also tweaked the documentation slightly to connect with the other set functions as Related Functions and some minor tweaks to the wording.

In the process of testing this I noticed that the upstream cty library, from which we're taking this function, has a bug: its SetSymmetricDifferenceFunc is actually implementing SetSubtractFunc, and thus producing the wrong answer. I've opened zclconf/go-cty#27 for that, but I've run out of time for now so I won't be able to make an upstream fix for that, and sadly that upstream fix will block us from merging this PR. If you or somebody else is interested in taking on the go-cty fix then I'd love to review a PR over in that repository.

We also sadly won't be able to merge this until you go through the CLA signing step described by @hashicorp-cla above. If that process is giving you some trouble please let me know and I will investigate.

Thanks again!

@apparentlymart apparentlymart self-assigned this Nov 12, 2019
@jjshoe
Copy link
Author

jjshoe commented Nov 12, 2019

@apparentlymart If I don't complete the CLA then if effectively blocks this feature from ever being added, as I can claim copyright, correct? What's wrong with the license that terraform uses that requires a cla?

Side note: I don't sign cla's. Unless you're willing to pay for my lawyer.

@apparentlymart
Copy link
Member

Thanks for letting us know, @jjshoe. In that case, I will close this out.

You can find information in the link that @hashicorp-cla posted above about why HashiCorp requires CLA agreement for contributions.

Thanks again for working on this!

@jjshoe
Copy link
Author

jjshoe commented Nov 12, 2019

@apparentlymart in this case I maintain my copyright, and any use, full or in partial, of the above code, is not permitted.

@ghost
Copy link

ghost commented Mar 28, 2020

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@ghost ghost locked and limited conversation to collaborators Mar 28, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
config enhancement sdkv1 [PRs only] Marks changes that may potentially need to be ported to the plugi nSDK
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants