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.

Sign up for GitHub

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

Jump to bottom

Add Golang wrapper APIs and tests #220

Merged
merged 5 commits into from
May 25, 2023
Merged

Add Golang wrapper APIs and tests #220

merged 5 commits into from
May 25, 2023

Conversation

marcelamelara
Copy link
Contributor

@marcelamelara marcelamelara commented May 10, 2023 •
edited
Loading

This PR introduces basic wrapper APIs for the Golang language bindings for the Statement layer. I also adds basic tests and documentation for testing the Golang language bindings.

Fixes #175

@marcelamelara marcelamelara requested a review from a team as a code owner May 10, 2023 16:59
adityasaky
adityasaky reviewed May 10, 2023
View reviewed changes
Copy link
Member

@adityasaky adityasaky left a comment

Choose a reason for hiding this comment

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

Nice! Some general comments / thoughts.

go/v1/statement.go Outdated Show resolved Hide resolved
go/v1/statement.go Outdated Show resolved Hide resolved
go/v1/statement.go Show resolved Hide resolved
go/v1/resource_descriptor_test.go Outdated Show resolved Hide resolved
adityasaky
adityasaky reviewed May 10, 2023
View reviewed changes
go/v1/resource_descriptor.go Outdated Show resolved Hide resolved
go/v1/resource_descriptor_test.go Outdated Show resolved Hide resolved
go/v1/statement.go Outdated Show resolved Hide resolved
go/v1/statement_test.go Outdated Show resolved Hide resolved
@marcelamelara marcelamelara force-pushed the add-go-apis branch 4 times, most recently from 8ea5cb0 to cb58730 Compare May 10, 2023 21:34
@marcelamelara marcelamelara mentioned this pull request May 11, 2023
Closed
3 tasks
migmartri
migmartri reviewed May 12, 2023
View reviewed changes
go/v1/statement.go Outdated Show resolved Hide resolved
migmartri
migmartri reviewed May 12, 2023
View reviewed changes
go/v1/statement_test.go Outdated Show resolved Hide resolved
@marcelamelara marcelamelara force-pushed the add-go-apis branch 3 times, most recently from eab85b2 to be76eec Compare May 12, 2023 21:12
marcelamelara and others added 4 commits May 12, 2023 15:05
@marcelamelara @adityasaky
Add Golang validator APIs for statement layer
76d1598 
Signed-off-by: Marcela Melara <[email protected]>

Co-authored-by: Aditya Sirish <[email protected]>
@marcelamelara @adityasaky
Add basic Golang tests for Statement layer
e55e200 
Signed-off-by: Marcela Melara <[email protected]>

Co-authored-by: Aditya Sirish <[email protected]>
@marcelamelara
Document Golang testing
80f5b13 
Signed-off-by: Marcela Melara <[email protected]>
@marcelamelara
Add workflow for automated implementation testing
e99d362 
Signed-off-by: Marcela Melara <[email protected]>
migmartri
migmartri reviewed May 13, 2023
View reviewed changes
Copy link

@migmartri migmartri left a comment

Choose a reason for hiding this comment

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

Thanks for applying some of the feedback! This looks great to me!

I'll leave the approval to some of the maintainers :)

LGTM!

TomHennen
TomHennen requested changes May 13, 2023
View reviewed changes
Copy link
Contributor

@TomHennen TomHennen left a comment

Choose a reason for hiding this comment

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

Thanks for this work Marcela, it seems pretty valuable!

go/v1/resource_descriptor.go Outdated Show resolved Hide resolved
go/v1/statement.go Outdated Show resolved Hide resolved
@adityasaky adityasaky mentioned this pull request May 15, 2023
Merged
@marcelamelara marcelamelara requested a review from TomHennen May 15, 2023 20:24
@TomHennen
Copy link
Contributor

Thanks Marcela!

@marcelamelara
Copy link
Contributor Author

Just going to note: We may be able to replace per-language validators with something more portable in the future.

@marcelamelara marcelamelara requested a review from a team May 24, 2023 17:41
pxp928
pxp928 approved these changes May 25, 2023
View reviewed changes
Copy link
Member

@pxp928 pxp928 left a comment

Choose a reason for hiding this comment

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

LGTM. A small nit that can be improved later if needed if more error messages are possible. Thanks for all the work!

go/v1/resource_descriptor_test.go Show resolved Hide resolved
@pxp928 pxp928 merged commit c7dc920 into in-toto:main May 25, 2023
This was referenced May 26, 2023
Closed
Open
@marcelamelara marcelamelara deleted the add-go-apis branch July 6, 2023 22:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@migmartri migmartri migmartri left review comments

@adityasaky adityasaky adityasaky left review comments

@TomHennen TomHennen TomHennen approved these changes

@pxp928 pxp928 pxp928 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Create test suite for Go bindings
5 participants
@marcelamelara @TomHennen @migmartri @adityasaky @pxp928