-
Notifications
You must be signed in to change notification settings - Fork 218
feat: add ability to store package documentation #4393
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
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: Austin Abro <[email protected]>
✅ Deploy Preview for zarf-docs canceled.
|
Codecov Report❌ Patch coverage is
... and 10 files with indirect coverage changes 🚀 New features to boost your workflow:
|
Signed-off-by: Austin Abro <[email protected]>
Signed-off-by: Austin Abro <[email protected]>
Signed-off-by: Austin Abro <[email protected]>
Signed-off-by: Austin Abro <[email protected]>
Signed-off-by: Austin Abro <[email protected]>
Signed-off-by: Austin Abro <[email protected]>
Signed-off-by: Austin Abro <[email protected]>
Signed-off-by: Austin Abro <[email protected]>
Signed-off-by: Austin Abro <[email protected]>
Signed-off-by: Austin Abro <[email protected]>
Signed-off-by: Austin Abro <[email protected]>
Signed-off-by: Austin Abro <[email protected]>
Signed-off-by: Austin Abro <[email protected]>
Signed-off-by: Austin Abro <[email protected]>
Signed-off-by: Austin Abro <[email protected]>
Signed-off-by: Austin Abro <[email protected]>
Signed-off-by: Austin Abro <[email protected]>
Signed-off-by: Austin Abro <[email protected]>
Signed-off-by: Austin Abro <[email protected]>
Signed-off-by: Austin Abro <[email protected]>
Signed-off-by: Austin Abro <[email protected]>
Signed-off-by: Austin Abro <[email protected]>
8672668 to
017bb1a
Compare
brandtkeller
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Couple passing thoughts but otherwise clean and straight-forward.
| description: Simple example to load classic DOS games into K8s in the airgap | ||
| version: 1.2.0 | ||
|
|
||
| # The documentation key can be utilized to pass information to package consumers |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this addition warrant bumping the package version (and possibly publishing)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we had a smoother workflow I think potentially, but currently it requires running a workflow to update the new version then updating our e2e tests that pull this package to use the new version, which can be cumbersome.
src/pkg/lint/validate.go
Outdated
| return err | ||
| } | ||
|
|
||
| func validateDocumentation(documentation map[string]string) error { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reasonable stance to take - though I can imagine we will see few examples in the future.
This stance is based on the storage mechanism in the package based on the assemble logic... Is it worth considering a broader copy strategy that prepends the key in order to establish more uniqueness?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My consideration was more so for the user when they get their documents. Keeping them unique makes it so their files are the same name when they read them. Though it is still probably best to prepend the filename with the key.
Signed-off-by: Austin Abro <[email protected]>
Signed-off-by: Austin Abro <[email protected]>
Signed-off-by: Austin Abro <[email protected]>
Signed-off-by: Austin Abro <[email protected]>
Description
Adds the ability to store documentation under the documentation key which is a map[string]string. Stores documentation files in the package by their base filename.
I decided not to include documentation from imported packages. If a user wants documentation from a package they are importing, it should be simple enough to copy that file and import the file separately, and I believe there are situations where documentation for an imported package, wouldn't make sense in the parent package.
Currently this feature does not allow pulling in remote files, I'd be open to adding this in the future if someone presents a use case.
One neat thing I learned is that if a user tries to define the same key twice in a yaml map then we will error, so no additional validation was needed for that.
Related Issue
Fixes #4346
Checklist before merging