Many workflows depend on hooks to enforce certain policies, e.g. to
prevent forced pushes. The Mercurial Guide includes some cases and
Google can help finding others, but it can save users a lot of time
if hg itself has a couple of examples for further customization.
Details
- Reviewers
baymax durin42 - Group Reviewers
hg-reviewers - Commits
- rHG4cabeea6d214: hgext: start building a library for simple hooks
Diff Detail
- Repository
- rHG Mercurial
- Branch
- default
- Lint
No Linters Available - Unit
No Unit Test Coverage
Event Timeline
Thanks a lot for putting efforts here. I am +1 on this. I haven't looked at the code though, will try to review if no one else beats me to it.
I think we should go a step further and have some built-in hooks in Mercurial itself, like we do extensions. A good first implementation would be to define those hooks somewhere where they can be imported, add tests like they are standalone hooks. As a follow-up (read: slightly more work), we can define a new config syntax for using these hooks. e.g. builtin:reject_new_heads.
What do others think?
I like where this is going, but I wonder if these specific hooks can be written without being in-process Python hooks. Reason being that Python hooks use the unstable internals of hg and are semi-discouraged if they're avoidable.
Thoughts?
If you do stick with in-process for this, I definitely want a test included so we don't regress our own examples. ;)
This is exactly why I think the Python hooks should be part of the modules shipped as part of the core distribution :)
There seems to have been no activities on this Diff for the past 3 Months.
By policy, we are automatically moving it out of the need-review state.
Please, move it back to need-review without hesitation if this diff should still be discussed.
:baymax:need-review-idle:
hgext/hooklib/__init__.py | ||
---|---|---|
7 | Assuming this is related to the IRC page. When the demand importer is active, an import will materialize a proxy module object: the code in that module won't get executed until an attribute of the module is accessed. So anything relying on run-on-import side-effects may not work correctly. Since our configuration option registration mechanism relies on code running as a side-effect of module import, in order to ensure this code runs, you need to ensure the module is "loaded" by accessing any of its attributes. In addition, the extension mechanism may be looking for a configtable attribute in the module in order to associate config options with extensions. If this isn't actually happening (I'm too lazy to look at the source code), then you won't need to assign to a local variable here: simply accessing changeset_published.configtable will be sufficient to trigger module execution and register the config options. |
I'm happy with this, modulo my one question for other maintainers. If anyone else is happy, please feel encouraged to push it.
hgext/hooklib/__init__.py | ||
---|---|---|
2 | Should we mark this as (EXPERIMENTAL) or are we comfortable holding on to this long-term? |
hgext/hooklib/__init__.py | ||
---|---|---|
2 | I'm perfectly fine with adding such a marker and a comment that the functionality is meant for long-term support, but e.g. might migrate into the notify extension. One of the big goals here is to ensure in-tree testing after all. |
hgext/hooklib/__init__.py | ||
---|---|---|
2 | I'd prefer to mark it experimental to start with. |
Should we mark this as (EXPERIMENTAL) or are we comfortable holding on to this long-term?
/cc @marmoute @martinvonz @indygreg @yuja