Conversation
robko23
left a comment
robko23
left a comment
There was a problem hiding this comment.
Hi, this is very nice addition, however i have a few comments/questions about the number of hooks. What do you think?
| for proj_id in &self.proj_writes { | ||
| if let Some((root, hooks)) = args.hooks.get(proj_id) { | ||
| hooks.execute_pre_begin(root)?; | ||
| } | ||
| } | ||
|
|
||
| for proj_id in &self.proj_writes { | ||
| if let Some((root, hooks)) = args.hooks.get(proj_id) { | ||
| hooks.execute_pre_write(root)?; | ||
| } | ||
| } |
There was a problem hiding this comment.
Do we really need two hooks that are in the same place but different name?
There was a problem hiding this comment.
It does seem a little weird, but the idea is that the hooks are semantically different. Long term goal would be that the pre-begin hook runs on all projects on every release, whereas the pre-write hook runs only on projects that actually will write something. Also, if there later exists some hook-able action that occurs before write, the pre-write hook will come after that, but pre-begin will still come before it. Finally, because hooks are all run together, this will give a user to have some control over when their hooks will run in relation to each other.
| for proj_id in &self.write.proj_writes { | ||
| if let Some((root, hooks)) = args.hooks.get(proj_id) { | ||
| hooks.execute_post_tag(root)?; | ||
| } | ||
| } | ||
|
|
||
| for proj_id in &self.write.proj_writes { | ||
| if let Some((root, hooks)) = args.hooks.get(proj_id) { | ||
| hooks.execute_post_end(root)?; | ||
| } | ||
| } |
2 participants
This adds additional hooks to the runtime, which (just like the existing
post_writehook) will not run on a dry run.Hooks added: