Skip to content

Do not set command name as part of Project*CommandRunner #5962

@lukemassa

Description

@lukemassa

Community Note

  • Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request. Searching for pre-existing feature requests helps us consolidate datapoints for identical requirements into a single place, thank you!
  • Please do not leave "+1" or other comments that do not add relevant new information or questions, they generate extra noise for issue followers and do not help prioritize the request.
  • If you are interested in working on this issue or have submitted a pull request, please leave a comment.

Describe the user story

Right now for each command there is a block like this:

func (p *DefaultProjectCommandRunner) Apply(ctx command.ProjectContext) command.ProjectResult {
	applyOut, failure, err := p.doApply(ctx)
	return command.ProjectResult{
		Command:           command.Apply,
		Failure:           failure,
		Error:             err,
		ApplySuccess:      applyOut,
		RepoRelDir:        ctx.RepoRelDir,
		Workspace:         ctx.Workspace,
		ProjectName:       ctx.ProjectName,
		SilencePRComments: ctx.SilencePRComments,
	}
}

The runner is "inventing" the name of the command that it assumed called it. Higher up in the call stack the command is known, then we "forget it" and sneak it back in here. This causes bugs like #5934, and in general doesn't make a lot of sense. In addition the ctx.* content (which again is returned by all the analogous commands) is a code smell here; ctx is being passed in to this function, the caller clearly knows things like ProjectName or Workspace, why are we telling it?

The issue is that the type ProjectResult is used in many places, and here is doing double duty of summarizing what happened in a given run, as well as the output from a given command.

Describe the solution you'd like

These functions should return a pared down ProjectCommandOutput, that the caller of this function should then "decorate" with the additional information.

Describe the drawbacks of your solution

It's a refactor so there's some risk there, but there's a lot of test coverage so should be ok.

Describe alternatives you've considered

I can't think of any

Metadata

Metadata

Assignees

No one assigned

    Labels

    featureNew functionality/enhancement

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions