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 impl TryFrom<&[u8; 48]> for QuoteHeader #13

Merged
merged 2 commits into from
Dec 17, 2020

Conversation

lkatalin
Copy link
Contributor

@lkatalin lkatalin commented Dec 16, 2020

This PR has

  • Splits Quote type into submodules (includes a few small fixes)
  • An impl TryFrom<&[u8; 48]> for QuoteHeader

The former involved some reorganization of the code in quote.rs to be more organized, ex. the types now show up in the order that they go into the Quote. There are also header comments to help delineate these different sections. Alternatively, I could break these parts out into small submodules in separate files. Any thoughts on this?
Edit: there are now submodules for quote.

Related to: enarx/enarx#917 ,
Related to: enarx/enarx#84

Copy link

@connorkuehl connorkuehl left a comment

Choose a reason for hiding this comment

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

Regarding the first commit: Instead of comment fences, would you consider grouping these types with submodules?

@lkatalin
Copy link
Contributor Author

@connorkuehl Yeah, sure! I'll push a new version.

@connorkuehl
Copy link

Should the module structure be:

quote
├── header
└── sigdata

@lkatalin lkatalin force-pushed the quoteheader branch 2 times, most recently from f5ee144 to 91041ec Compare December 17, 2020 02:11
This breaks quote.rs into smaller submodules quote.rs, quoteheader.rs,
and sigdata.rs.

Signed-off-by: Lily Sturmann <lsturman@redhat.com>
Signed-off-by: Lily Sturmann <lsturman@redhat.com>
@lkatalin
Copy link
Contributor Author

Should the module structure be:

quote
├── header
└── sigdata

@connorkuehl That is an extremely reasonable suggestion. Updated.

Copy link

@connorkuehl connorkuehl 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! 🙂

@enarxbot enarxbot merged commit cf6ea31 into enarx:master Dec 17, 2020
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.

6 participants