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.

Sign up for GitHub

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

Jump to bottom

fix(api): Fix type mismatch with Pipeline boolean properties #1046

Merged
merged 1 commit into from
Jul 23, 2021
Merged

fix(api): Fix type mismatch with Pipeline boolean properties #1046

merged 1 commit into from
Jul 23, 2021

Conversation

mochacat
Copy link
Contributor

Pipeline properties that were migrated to the front50-api package as booleans should actually be strings, since this was the previous behavior (ie true -> "true", false -> "false). This PR reverts these properties back.

edgarulg
edgarulg approved these changes Jul 23, 2021
View reviewed changes
Copy link
Contributor

@edgarulg edgarulg left a comment

Choose a reason for hiding this comment

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

LGTM

@edgarulg edgarulg merged commit b082bae into spinnaker:master Jul 23, 2021
@mochacat mochacat deleted the fix-booleans-pipeline branch July 23, 2021 20:44
pemmasanikrishna pushed a commit to pemmasanikrishna/front50 that referenced this pull request Sep 21, 2021
@mochacat
fix(api): fix type mismatch regression with Pipeline boolean properti…
9e34459 
…es (spinnaker#1046)
@spinnakerbot spinnakerbot mentioned this pull request Sep 29, 2021
Merged
edgarulg pushed a commit that referenced this pull request Sep 29, 2021
@cristhian-castaneda
Revert "fix(api): fix type mismatch regression with Pipeline boolean …
66dc951 
…properties (#1046)" (#1068)

This reverts commit b082bae.
mergify bot pushed a commit that referenced this pull request Sep 29, 2021
@cristhian-castaneda
Revert "fix(api): fix type mismatch regression with Pipeline boolean …
270c680 
…properties (#1046)" (#1068)

This reverts commit b082bae.

(cherry picked from commit 66dc951)
@spinnakerbot spinnakerbot mentioned this pull request Sep 29, 2021
Merged
edgarulg pushed a commit that referenced this pull request Sep 29, 2021
@mergify @cristhian-castaneda
Revert "fix(api): fix type mismatch regression with Pipeline boolean …
bd3eb33 
…properties (#1046)" (#1068) (#1069)

This reverts commit b082bae.

(cherry picked from commit 66dc951)

Co-authored-by: Cristhian Castaneda <ccastanedarivera@gmail.com>
@smecsia
Copy link

smecsia commented Oct 25, 2021

Hi @mochacat @edgarulg . I might be missing something, but this statement booleans should actually be strings, since this was the previous behavior does not seem to be true? Can you please provide more context on why they have to be strings?

I'm looking at Deck definitions in master branch and all these fields have boolean type:
https://github.com/spinnaker/deck/blob/master/packages/core/src/domain/IPipeline.ts#L14
https://github.com/spinnaker/deck/blob/master/packages/core/src/domain/IPipeline.ts#L17
https://github.com/spinnaker/deck/blob/master/packages/core/src/domain/IPipeline.ts#L28

For us this change broke compatibility with frontend.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@edgarulg edgarulg edgarulg approved these changes

Assignees
No one assigned
Labels
target-release/1.27
Projects
None yet
Milestone
No milestone
4 participants
@mochacat @smecsia @edgarulg @spinnakerbot