Skip to content

scriptrun base and planning#5990

Merged
t-kikuc merged 3 commits into
pipe-cd:masterfrom
hiep-tk:scriptrun_pr
Jul 10, 2025
Merged

scriptrun base and planning#5990
t-kikuc merged 3 commits into
pipe-cd:masterfrom
hiep-tk:scriptrun_pr

Conversation

@hiep-tk
Copy link
Copy Markdown
Contributor

@hiep-tk hiep-tk commented Jul 2, 2025

What this PR does: Scriptrun base and planning phase

Why we need it: scriptrun stage migration to plugin

Which issue(s) this PR fixes: Part of #5901

Fixes #

Does this PR introduce a user-facing change?:

  • How are users affected by this change:
  • Is this breaking change:
  • How to migrate (if breaking change):

Copy link
Copy Markdown
Member

@t-kikuc t-kikuc left a comment

Choose a reason for hiding this comment

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

thanks, basically it's good.

Comment thread pkg/app/pipedv1/plugin/scriptrun/plugin.go
Comment thread pkg/app/pipedv1/plugin/scriptrun/options.go
stageScriptRunRollback = "SCRIPT_RUN_ROLLBACK"
)

type ContextInfo struct {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is not related to this PR. Let's add in another PR.

}, nil
}

func (p *plugin) FetchDefinedStages() []string {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

add test

Comment thread pkg/app/pipedv1/plugin/scriptrun/plugin.go
Comment thread pkg/app/pipedv1/plugin/scriptrun/plugin.go
@hiep-tk hiep-tk requested a review from t-kikuc July 3, 2025 07:00
Comment thread pkg/app/pipedv1/plugin/scriptrun/plugin_test.go
Comment thread pkg/app/pipedv1/plugin/scriptrun/plugin_test.go

func TestBuildPipelineSyncStages(t *testing.T) {
p := &plugin{}
ctx := context.Background()
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

you can use t.Context() instead

Comment thread pkg/app/pipedv1/plugin/scriptrun/plugin_test.go
Comment thread pkg/app/pipedv1/plugin/scriptrun/plugin_test.go
name string
input *sdk.BuildPipelineSyncStagesInput
expected *sdk.BuildPipelineSyncStagesResponse
expectErr bool
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

expectErr is not used.

Comment thread pkg/app/pipedv1/plugin/scriptrun/options.go
@t-kikuc t-kikuc self-assigned this Jul 3, 2025
@hiep-tk hiep-tk requested a review from t-kikuc July 4, 2025 02:23
// the command(s) to run
Run string `json:"run,omitempty"`
// timeout limit for this stage
Timeout config.Duration `json:"timeout,omitempty" default:"6h"`
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Use pkg/plugin/sdk/unit/duration.go instead. we created it recently.

Comment thread pkg/app/pipedv1/plugin/scriptrun/plugin_test.go
@t-kikuc t-kikuc enabled auto-merge (squash) July 8, 2025 06:19
Signed-off-by: hiep-tk <hiep.trinhkhanh0466@gmail.com>
hiep-tk added 2 commits July 8, 2025 15:52
Signed-off-by: hiep-tk <hiep.trinhkhanh0466@gmail.com>
Signed-off-by: hiep-tk <hiep.trinhkhanh0466@gmail.com>
Copy link
Copy Markdown
Member

@t-kikuc t-kikuc left a comment

Choose a reason for hiding this comment

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

Thank you 👍

@t-kikuc t-kikuc removed their assignment Jul 8, 2025
Copy link
Copy Markdown
Member

@khanhtc1202 khanhtc1202 left a comment

Choose a reason for hiding this comment

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

Thanks 👍

@t-kikuc t-kikuc merged commit ea05d31 into pipe-cd:master Jul 10, 2025
39 checks passed
@github-actions github-actions Bot mentioned this pull request Jul 14, 2025
@github-actions github-actions Bot mentioned this pull request Aug 15, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants