Skip to content
This repository was archived by the owner on Mar 15, 2018. It is now read-only.

iarc v2 "client" (bug 1228376, 1228374)#3459

Closed
ngokevin wants to merge 4 commits intomozilla:masterfrom
ngokevin:iarcv2
Closed

iarc v2 "client" (bug 1228376, 1228374)#3459
ngokevin wants to merge 4 commits intomozilla:masterfrom
ngokevin:iarcv2

Conversation

@ngokevin
Copy link
Copy Markdown
Contributor

No description provided.

@ngokevin ngokevin changed the title iarc v2 client (bug 1228376, 1228374) iarc v2 "client" (bug 1228376, 1228374) Nov 30, 2015
@ngokevin
Copy link
Copy Markdown
Contributor Author

failing test has a PR

@ngokevin ngokevin closed this Dec 1, 2015
@ngokevin ngokevin reopened this Dec 1, 2015
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

cert_uuid naming is not coherent with search_certs().

Also, I don't really like the action parameter, because it exposes to the outside world the underlying implementation.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Regarding my previous comment about action : My plan was for the client to use the serializer from #3457, being high-level enough to be used by the webapp code, hiding IARC implementation from them. Your implementation is closer to a super-thin client. Were you planning on using it directly in webapp models/tasks ?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

i can make it so it uses the serializer. and i like hiding action, i'll split it out

@diox
Copy link
Copy Markdown
Member

diox commented Dec 1, 2015

Move my serializer and its tests to the new iarc_v2 folder you created while you're at it.

@ngokevin
Copy link
Copy Markdown
Contributor Author

ngokevin commented Dec 1, 2015

the serializer will also be used by developers/views PushCerts. we can just have that import from lib/iarc_v2?

@diox
Copy link
Copy Markdown
Member

diox commented Dec 1, 2015

Yeah

@ngokevin
Copy link
Copy Markdown
Contributor Author

ngokevin commented Dec 2, 2015

@diox if it's not good by the time i'm online, feel free to fetch or merge and work off of it

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Call this cert_id to be coherent with the rest.

@diox
Copy link
Copy Markdown
Member

diox commented Dec 16, 2015

Superseded by #3466

@diox diox closed this Dec 16, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants