Skip to content
This repository has been archived by the owner on Apr 13, 2020. It is now read-only.

Fix repository name not populated #496

Merged
merged 2 commits into from
Apr 2, 2020
Merged

Fix repository name not populated #496

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6c81421
Fix repository name not populated
Apr 2, 2020
99b6904
Update pipeline.ts
dennisseah Apr 2, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/commands/project/pipeline.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ export const fetchValidateValues = (
orgName: opts.orgName || "",
Copy link
Collaborator

@mtarng mtarng Apr 2, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

This whole logic section is kinda confusing. Why are we still loading from opts from here. Shouldn't all of these be loaded from the values object instead?

This was introduced in: https://github.com/CatalystCode/spk/pull/479/files

@edaena @dennisseah does this make sense for us to fix this all here? Does similar logic for other ConfigValues interfaces exist now too in other commands?

Copy link
Collaborator

Choose a reason for hiding this comment

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

@mtarng do you know other places that this problem too?

Copy link
Collaborator

@mtarng mtarng Apr 2, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

I'm not sure, which is why I asked the question @dennisseah . Have we refactored any other commands to move arguments from CommandOptions to ConfigValues?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I will double check

buildScriptUrl: opts.buildScriptUrl || BUILD_SCRIPT_URL,
devopsProject: opts.devopsProject || "",
repoName: opts.repoName || "",
repoName: values.repoName,
personalAccessToken: opts.personalAccessToken || "",
pipelineName: opts.pipelineName || "",
repoUrl: opts.repoUrl || "",
Expand Down