Always template vendor source and targets #712
Conversation
This change improves the templating within vendor manifests slightly: It officially adds support for the `Component` field to both `source` and `targets`. These features were already supported but mostly undocumented and hidden behind an implicit switch: The templating was only triggered if the `Version` field was set. Which was also the only officially supported field. In reality though all fields from the current source definition were available but in the state they were currently in, depending on the order of the templates. With this change * It is clearly documented which fields are supported in which YAML values. * Only the two static fields are supported. * The values are always templated. Theoretically this could be a breaking change if somebody used no `version` field but curly braces in their paths. Or relied on the half-populated source data structure to refer to unsupported fields. If xkcd 1172 applies it should be possible to amend this logic to add more officially supported fields.
e1fd706 to
270fdd8
Compare
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
📝 WalkthroughWalkthroughThe changes involve enhancements to the handling of vendor sources and targets in the Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Outside diff range comments (2)
website/docs/core-concepts/vendor/vendor-manifest.mdx (2)
Line range hint
201-213: Excellent enhancement to templating documentation!The addition of detailed explanations for Go templates and Sprig Functions, along with the advanced example, significantly improves the documentation. This aligns perfectly with the PR objectives of clarifying supported fields and enhancing templating capabilities.
To further improve user understanding, consider adding a brief explanation of what the advanced example does. For instance, you could add a comment like:
targets: # Vendor a component into a major-minor versioned folder (e.g., "1.2" for version "1.2.3") - "components/terraform/infra/vpc-flow-logs-bucket/{{ (first 2 (splitList \".\" .Version)) | join \".\" }}"This would help users quickly grasp the purpose of this complex templating example.
Line range hint
1-524: Consider adding a note about potential breaking changesThe documentation updates are comprehensive and well-explained. However, to fully align with the PR objectives and ensure a smooth transition for users, consider adding a note about potential breaking changes.
Suggest adding a callout or warning section that mentions:
- This change could potentially break existing configurations that relied on the absence of a 'version' field.
- Configurations using unsupported fields in their paths may need to be updated.
This addition would help users anticipate and address any issues when updating to the new version, aligning with the PR's goal of clear communication about the changes.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
📒 Files selected for processing (2)
- internal/exec/vendor_utils.go (2 hunks)
- website/docs/core-concepts/vendor/vendor-manifest.mdx (3 hunks)
🧰 Additional context used
🪛 LanguageTool
website/docs/core-concepts/vendor/vendor-manifest.mdx
[style] ~96-~96: To make your text as clear as possible to all readers, do not use this foreign term. Possible alternatives are “below” or “further on” (in a document).
Context: ...argets: - "components/terraform/infra/{{.Component}}/{{.Version}}" excl...(INFRA)
🔇 Additional comments (2)
website/docs/core-concepts/vendor/vendor-manifest.mdx (1)
Line range hint
1-524: Excellent documentation updates aligned with PR objectivesThe changes throughout this file consistently support the PR objectives:
- Enhanced templating capabilities are clearly explained, including support for '{{.Component}}' and '{{.Version}}' in both 'source' and 'targets' fields.
- The documentation now provides better visibility of existing features, addressing the author's initial difficulty in finding relevant examples.
- The additions clarify which fields are supported in the YAML values, improving overall user understanding.
These updates significantly enhance the documentation's clarity and usefulness, making it easier for users to understand and implement the vendoring features in Atmos.
internal/exec/vendor_utils.go (1)
285-289: LGTM!The
tmplDatastruct is correctly initialized withs.Componentands.Version.
|
These changes were released in v1.90.0. |
|
@mss can you join us in SweetOps slack in slack.cloudposse.com |
the |
what
This change improves the templating within vendor manifests slightly: It officially adds support for the
Componentfield to bothsourceandtargets.These features were already supported but mostly undocumented and hidden behind an implicit switch: The templating was only triggered if the
Versionfield was set. Which was also the only officially supported field.In reality though all fields from the current source definition were available but in the state they were currently in, depending on the order of the templates.
With this change
Theoretically this could be a breaking change if somebody used no
versionfield but curly braces in their paths. Or relied on the half-populated source data structure to refer to unsupported fields. If xkcd 1172 applies it should be possible to amend this logic to add more officially supported fields.why
I was looking for a way to restructure our vendoring like the examples in
examples/demo-vendoring/vendor.yamlto avoid copy and paste errors when we release new component versions.I actually only found out about that demo when I was done writing this code since the templating was never triggered without a
versionfield and the documentation didn't mention it.references
Summary by CodeRabbit
New Features
vendor.yaml.sourceandtargetsattributes for better organization and flexibility.Documentation
included_pathsandexcluded_pathsattributes to improve understanding.