Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(controller): Revert parameter value to *string. Fixes #3960 #3963

Merged
merged 5 commits into from
Sep 11, 2020

Conversation

alexec
Copy link
Contributor

@alexec alexec commented Sep 8, 2020

Checklist:

  • Either (a) I've created an enhancement proposal and discussed it with the community, (b) this is a bug fix, or (c) this is a chore.
  • The title of the PR is (a) conventional, (b) states what changed, and (c) suffixes the related issues number. E.g. "fix(controller): Updates such and such. Fixes #1234".
  • I've signed the CLA.
  • I have written unit and/or e2e tests for my change. PRs without these are unlikely to be merged.
  • My builds are green. Try syncing with master if they are not.
  • My organization is added to USERS.md.

Fixes #3960

IntStr is a misnomer. It cannot handle integer values (specifically int64). This PR reverts to v2.9

@alexec alexec added this to the v2.10 milestone Sep 8, 2020
@alexec alexec added type/bug type/regression Regression from previous behavior (a specific type of bug) labels Sep 8, 2020
@alexec alexec requested a review from simster7 September 8, 2020 15:22
@alexec alexec changed the title fix(controller): Revert parameter value to *string. Fixes #3960 fix(controller): Revert parameter value to *string. Fixes #3960 Sep 8, 2020
@alexec alexec marked this pull request as ready for review September 8, 2020 18:42
@@ -610,11 +610,11 @@ type Parameter struct {
Name string `json:"name" protobuf:"bytes,1,opt,name=name"`

// Default is the default value to use for an input parameter if a value was not supplied
Default *intstr.IntOrString `json:"default,omitempty" protobuf:"bytes,2,opt,name=default"`
Default *string `json:"default,omitempty" protobuf:"bytes,2,opt,name=default"`
Copy link
Member

Choose a reason for hiding this comment

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

I think this value is wholly unused. Can we remove it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I thought that, but I actually think it's used in some weird and non-obvious dynamic way .

@alexec
Copy link
Contributor Author

alexec commented Sep 10, 2020

@jessesuen can you please approve this PR? It is a revert to v2.9 code and needs to be back-ported to v2.10 and v2.11.

@alexec alexec merged commit fd1465c into argoproj:master Sep 11, 2020
@alexec alexec deleted the rm-is branch September 11, 2020 19:33
@alexec alexec modified the milestones: v2.10, v2.11 Sep 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/bug type/regression Regression from previous behavior (a specific type of bug)
Projects
None yet
3 participants