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

KU and EKU Inconsistent lint correction #528

Merged
merged 18 commits into from
Jan 2, 2021

Conversation

ZsofiaTomicsko
Copy link
Contributor

Hi!

#497 was closed before I could add the last 2 commits.

  1. I have removed the comments explaining the previously removed functions and added new ones
  2. I added the tests for the truth tables

@christopher-henderson I have changed a small detail in your code that I think was a typo:

func (accepted Accepted) Xor(truther Truther) Accepted {
	if truther == nil {
		return accepted
	}
	switch other := truther.(type) {
	case KeyUsage:
		for key := range accepted {
			accepted[key|other] = true
		}
		accepted[other] = true
	case Accepted:
		for key := range other {
			accepted.Xor(key)
		}
	}
	return accepted
}

In the case of the input being a KeyUsage I removed:

for key := range accepted {
			accepted[key|other] = true
		}

I didn't know where to put the xor and or functions so I just copied them inside the test file too. Should I maybe move them elsewhere?

@christopher-henderson
Copy link
Member

Thank you for revisiting this!

I didn't know where to put the xor and or functions so I just copied them inside the test file too. Should I maybe move them elsewhere?

You're right about the bug in my code and honestly the test is more complex itself, so I think keeping it private to this file is for the best as it's really not battle hardened for general use.

@christopher-henderson
Copy link
Member

@zakird ready to merge these truth table tests (say that ten time fast).

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.

4 participants