Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Result hooks #304
Result hooks #304
Changes from 4 commits
bdf6a54
2db2a65
532341c
635dc2c
7186e8e
685f634
694b34f
f64b7a6
94c0ad2
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
I know this was me, but it is a stupid example ;-)
What about replacing the command with
cp ~/templates/standard-readme.txt {path}/README
and sell it as automatically populate a dataset with a default README.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.
(Personally, I found the example hilarious ;-) Will change it nevertheless)
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.
I fail to get such a hook to run because of an inconveniently coinciding mismatching formatting options and command requirements: I am trying to execute "
datalad run --output "README" "cp ~/Templates/standard-readme.txt {path}/README" --explicit
" via a hook after successful installation of a dataset. Here are mygit config
calls:For matching:
git config --global --add datalad.result-hook.readme.match-json '{"type": "dataset","action":"create","status":"ok"}'
Hook definition:
git config --global --add datalad.result-hook.readme.call-json 'run {{"cmd":"cp ~/Templates/standard-readme.txt {path}/README", "outputs":"["README"]", "dataset":"{path}","explicit":true}}'
The important part here is
"outputs":"["README"]"
. I need to give the output definition as a list with the string. I thought I once PR'ed anassure_list()
, but could only find this forcreate
...This fails with (Debug output):
Because in
"outputs":"["README"]"
, only the"["
part is considered. Switching to single quotes ("outputs":"['README']"
) or leaving quotation marks completely (both lead to no quotes in the config file) leads toREADME
being split into its components:I've unsucessfully tried escaping single or double quotes in the config call as well (e.g.,
git config --global --replace-all datalad.result-hook.readme.call-json 'run {{"cmd":"cp ~/Templates/standard-readme.txt {path}/README", "outputs":"[\'README\']", "dataset":"{path}","explicit":true}}'
. Do you see a way of giving a list with a string to the hook definition @mih?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.
I will try it myself, but why
"["README"]"
instead of["README"]
. This is valid JSON for a list.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.
Seems to work fine without the outer quotes.