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

DRY attestation statement accessor methods #124

Merged
merged 3 commits into from
Apr 23, 2019

Conversation

bdewater
Copy link
Collaborator

@bdewater bdewater commented Mar 5, 2019

While working on Android Key attestation format support I noticed I was writing some of these again, a quick scan of the spec showed me these all are used by at least two formats (often more) so it made sense to deduplicate these.

@grzuy
Copy link
Contributor

grzuy commented Mar 5, 2019

Hi @bdewater,

Interesting.

To me it doesn't seem like "too much" repetition going on, at least as of right now.
Even some of those are not yet repeating elsewhere.

Also, not sure about making some attestation statements respond to methods that doesn't make sense to them.

In summary, I am not sure about this, at least at this moment. But I think it would be a good idea to revisit when more repetition appears definitely.

Thank you for bringing this up and opening the PR!

@grzuy
Copy link
Contributor

grzuy commented Apr 22, 2019

Hi @bdewater,

I re-visited this and changed my mind.
Even added an extra commit to DRY a tiny bit more.

@grzuy grzuy merged commit 21afc84 into cedarcode:master Apr 23, 2019
@grzuy
Copy link
Contributor

grzuy commented Apr 23, 2019

Thanks again

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.

None yet

2 participants