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

Added documentation to Context methods. #168

Merged
merged 1 commit into from
Jan 7, 2021

Conversation

Superhepper
Copy link
Collaborator

  • Added documentation to the Context methods so that
    they at least have some small documentation.

Signed-off-by: Jesper Brynolf jesper.brynolf@gmail.com

@Superhepper Superhepper force-pushed the documentation branch 2 times, most recently from ef8850a to 69e1d12 Compare December 18, 2020 12:14
Copy link
Member

@ionut-arm ionut-arm left a comment

Choose a reason for hiding this comment

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

LGTM, are you intending to add references to the spec?

@Superhepper
Copy link
Collaborator Author

I am not sure. I might. But most of all I am going to add some examples before this is done.

@Superhepper Superhepper force-pushed the documentation branch 3 times, most recently from 6cb4e59 to b1fa8a4 Compare December 27, 2020 14:52
src/context.rs Outdated Show resolved Hide resolved
src/context.rs Outdated Show resolved Hide resolved
@Superhepper Superhepper force-pushed the documentation branch 3 times, most recently from 24a27aa to e763a55 Compare December 29, 2020 15:38
@Superhepper
Copy link
Collaborator Author

Do you think the links to the specifications are a good thing?

@puiterwijk
Copy link
Collaborator

Do you think the links to the specifications are a good thing?

I don't think that that makes a lot of sense. The Context layer is pretty hard to use for the non-initiated, and the functions are named close enough for the initiated to find them I think.
The abstraction layer should be such that you don't need the specifications, and then sending people there would be detrimental to adoption, I think, as it would mean that they get sent to highly complicated documents to use simple things.

@puiterwijk
Copy link
Collaborator

I think that we could do with a higher level crates that are more of the abstraction type of code, but I think that that should be separate endeavours.
I've already done that at some places, like e.g. https://github.com/fedora-iot/rust-tpm2-policy for policy abstraction (though for now it's still lacking a lot of policies, and documentation).

@Superhepper Superhepper force-pushed the documentation branch 2 times, most recently from e84958f to c7f23e8 Compare January 5, 2021 21:46
Copy link
Member

@ionut-arm ionut-arm left a comment

Choose a reason for hiding this comment

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

I'm ok with it either way, whether the references to the spec are there or not, I'm just glad we have code examples now

@puiterwijk
Copy link
Collaborator

I'm ok with it either way, whether the references to the spec are there or not, I'm just glad we have code examples now

Oh, absolutely! Thanks @Superhepper for the code examples and documentation!

@Superhepper
Copy link
Collaborator Author

I think this PR is finished now. Even though the documentation of Context is not finished it contains a couple a good examples so it is easy for others to continue. And I do not want the PR to be to big.

- Added documentation to the Context methods so that
  they at least have some small documentation.

Signed-off-by: Jesper Brynolf <jesper.brynolf@gmail.com>
@Superhepper Superhepper marked this pull request as ready for review January 7, 2021 13:30
Copy link
Collaborator

@puiterwijk puiterwijk left a comment

Choose a reason for hiding this comment

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

Looks good to me! Thanks!

@Superhepper Superhepper merged commit db79e83 into parallaxsecond:master Jan 7, 2021
@Superhepper Superhepper deleted the documentation branch September 10, 2021 15:48
tgonzalezorlandoarm pushed a commit to tgonzalezorlandoarm/rust-tss-esapi that referenced this pull request Mar 14, 2024
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.

3 participants