Skip to content

Add GH_OST_INSTANT_DDL for gh-ost-on-success hook#1658

Merged
meiji163 merged 2 commits intomasterfrom
meiji163/instantddl-hook-env-var
Apr 8, 2026
Merged

Add GH_OST_INSTANT_DDL for gh-ost-on-success hook#1658
meiji163 merged 2 commits intomasterfrom
meiji163/instantddl-hook-env-var

Conversation

@meiji163
Copy link
Copy Markdown
Contributor

@meiji163 meiji163 commented Apr 8, 2026

Description

This PR adds boolean environment variable GH_OST_INSTANT_DDL for gh-ost-on-success hook, so one can tell whether gh-ost used instant DDL successfully

In case this PR introduced Go code changes:

  • contributed code is using same conventions as original code
  • script/cibuild returns with no formatting errors, build errors or unit test errors.

Copilot AI review requested due to automatic review settings April 8, 2026 17:42
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds a new boolean environment variable (GH_OST_INSTANT_DDL) to the gh-ost-on-success hook so hook scripts can determine whether the migration completed via Instant DDL.

Changes:

  • Extend HooksExecutor.onSuccess() to accept an instantDDL boolean and export it as GH_OST_INSTANT_DDL to success hooks.
  • Wire the new onSuccess(true/false) calls from the migrator’s success paths (instant vs non-instant).
  • Document GH_OST_INSTANT_DDL in the hooks documentation.
Show a summary per file
File Description
go/logic/migrator.go Passes instantDDL into the success hook and adds a noop/instant-ddl early return (currently problematic).
go/logic/hooks.go Adds GH_OST_INSTANT_DDL env var injection for gh-ost-on-success hooks.
doc/hooks.md Documents the new GH_OST_INSTANT_DDL variable for the success hook.

Copilot's findings

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

  • Files reviewed: 3/3 changed files
  • Comments generated: 3

@meiji163 meiji163 merged commit 0270a28 into master Apr 8, 2026
9 checks passed
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.

3 participants