-
Notifications
You must be signed in to change notification settings - Fork 11
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 addPath
option
#57
Conversation
Signed-off-by: Saswata Mukherjee <[email protected]>
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.
Thanks for this work, but not sure completely about this.
@@ -68,6 +68,10 @@ type TransformationConfig struct { | |||
// TODO(bwplotka): Explain ** and * suffixes and ability to specify "invalid" paths like "/../". | |||
Path string | |||
|
|||
// Add is an optional path for a file to be created with certain frontMatter. | |||
// Only parses frontMatter and cannot be mentioned alongside path and glob. | |||
AddPath string `yaml:"addPath"` |
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.
Thanks for this work, but I think I am not a fan of this approach. This is a little bit mix of functionalities. We do go through transformations and we don't transform any old things here. Also why the only frontmatter if the whole file is empty? Also where the new file is put? In provided path? What if we want to provide a file next to another file we traverse?
I think it will be unwise to abstract all Linux commands within the transform configuration ): It's so easy to create this file with simple echo ... > file
.
One functionality which would be harder with Linux commands (harder than 3 words) is to add certain file content next to another file we traverse with glob 🤔
I think I would prefer something simple for now like ExtraFiles
with content
template or something like this 🤔
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.
Also why the only frontmatter if the whole file is empty?
Creating a new file, with just frontmatter, was the use case I was aiming to fulfill with this, as seen here in Thanos.
Also where the new file is put? In provided path? What if we want to provide a file next to another file we traverse?
Yes, it is put in the provided path. For putting a new file next to a particular file we traverse, I think we can just mention path of the file we want to place it next to...but yes, addPath
cannot work with glob.
I think it will be unwise to abstract all Linux commands within the transform configuration ): It's so easy to create this file with simple
echo ... > file
.
Yes, I see your point. Will try to make this simpler with ExtraFiles
and content
! 🙂
Closing as discussed in 1:1. 🙂 |
This PR adds
addPath
option totransform
config which allows adding a new file tooutputDir
withfrontMatter
using config like below,It cannot be specified alongside
path
orglob
.Only
Origin.LastMod
andtarget.FileName
are specifiable inaddPath
frontMatter template. Fixes #51.