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 metadata option to .manifest #4306

Merged

Conversation

marensws
Copy link
Contributor

fixes: #4289

bundle/bundle.go Outdated
@@ -180,7 +180,7 @@ func (m Manifest) Copy() Manifest {

func (m Manifest) String() string {
m.Init()
return fmt.Sprintf("<revision: %q, roots: %v, wasm: %+v>", m.Revision, *m.Roots, m.WasmResolvers)
return fmt.Sprintf("<revision: %q, roots: %v, wasm: %+v, metadata: %x>", m.Revision, *m.Roots, m.WasmResolvers, m.Metadata)
Copy link
Contributor

Choose a reason for hiding this comment

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

Change %x to %+v

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@marensws marensws force-pushed the add-metadata-option-to-.manifest branch 5 times, most recently from 1c7891b to fd17716 Compare January 31, 2022 09:58
Copy link
Contributor

@johanneslarsson johanneslarsson left a comment

Choose a reason for hiding this comment

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

LGTM

@marensws marensws force-pushed the add-metadata-option-to-.manifest branch 3 times, most recently from 0fd3ba2 to 6d969b9 Compare January 31, 2022 11:50
fixes: open-policy-agent#4289
Signed-off-by: marensws <msws@live.no>
@marensws marensws force-pushed the add-metadata-option-to-.manifest branch from f589f50 to 6c24ab7 Compare January 31, 2022 13:05
Copy link
Contributor

@srenatus srenatus left a comment

Choose a reason for hiding this comment

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

LGTM thanks a lot. There's a whitespace nitpick, but please ignore that. Let's merge this.

@@ -180,6 +181,12 @@ func (c *Compiler) WithCapabilities(capabilities *ast.Capabilities) *Compiler {
return c
}

//WithMetadata sets the additional data to be included in .manifest
Copy link
Contributor

Choose a reason for hiding this comment

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

[nit]

Suggested change
//WithMetadata sets the additional data to be included in .manifest
// WithMetadata sets the additional data to be included in .manifest

@srenatus srenatus merged commit a18f53d into open-policy-agent:main Jan 31, 2022
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.

compile.go missing option to add manifest.metadata
3 participants