Skip to content

Upgrade helper version for hook in version 2.1#2152

Open
Josue-T wants to merge 34 commits intodevfrom
hook_v21
Open

Upgrade helper version for hook in version 2.1#2152
Josue-T wants to merge 34 commits intodevfrom
hook_v21

Conversation

@Josue-T
Copy link
Contributor

@Josue-T Josue-T commented Aug 12, 2025

The problem

We still are using helper v2.0 for hooks

Solution

Use helpers v2.1.

Note I specified the helper version in each file to avoid to upgrade the hook version in case some apps still use helper v2.0 for hooks.

PR Status

Can be tested, seem working

How to test

Run the regen-conf, backup, restore

@Josue-T Josue-T requested a review from alexAubin August 12, 2025 20:43
@Josue-T Josue-T mentioned this pull request Sep 17, 2025
@Salamandar
Copy link
Contributor

This will break restoration of old backups.

@Josue-T
Copy link
Contributor Author

Josue-T commented Sep 17, 2025

should be mostly good except one thing, how to handle the backup dir because currently we use YNH_CWD to define the destination and as the core backup is called all in one by as a hook we can't set YNH_CWD for each hook.

@Josue-T
Copy link
Contributor Author

Josue-T commented Sep 17, 2025

This will break restoration of old backups.

Well, normally no because the change try to stay retro compatible. And the original path is saved in the CSV, so at the end in most of case we don't really take care of where we backup. But fee free to bring some improvement in this PR if you have a better idea of how to handle this.

@Josue-T Josue-T requested a review from zamentur November 14, 2025 20:22
Josue-T and others added 2 commits November 16, 2025 11:50
…idual_script' such that it's actually meaningful ... + trash unused post_callback arg for hook_callback
Copy link
Member

@alexAubin alexAubin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(LGTM modulo how happy the tests are)

@Josue-T Josue-T requested a review from alexAubin January 9, 2026 07:23
@Josue-T
Copy link
Contributor Author

Josue-T commented Jan 9, 2026

@alexAubin do you think we could merge it maybe in Trixie ?

Copy link
Member

@alexAubin alexAubin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm gonna say it looks okay to me to merge in current dev but we may get some surprises even though we pretty much carefully reviewed it several times

Any opinion @zamentur @Salamandar ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants