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

feat(spel): default new pipelines to spel v4 #7917

Merged
merged 4 commits into from
Feb 19, 2020
Merged

feat(spel): default new pipelines to spel v4 #7917

merged 4 commits into from
Feb 19, 2020

Conversation

marchello2000
Copy link
Contributor

now that spel v4 is live and glorious in orca let's make deck create new pipelines with that as default

@marchello2000
feat(spel): default new pipelines to spel v4
d3004fb 
now that spel v4 is live and glorious in orca let's make deck create new pipelines with that as default
marchello2000
marchello2000 commented Feb 19, 2020
View reviewed changes
app/scripts/modules/core/src/pipeline/create/CreatePipelineModal.tsx
@@ -389,10 +390,7 @@ export class CreatePipelineModal extends React.Component<ICreatePipelineModalPro
</div>
<div className="col-md-7">
<Select
options={[
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 guess the linter autofixed this?

Copy link
Member

Choose a reason for hiding this comment

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

It's true. Sometimes new Prettier versions disagree with older ones.

erikmunson
erikmunson approved these changes Feb 19, 2020
View reviewed changes
Copy link
Member

@erikmunson erikmunson left a comment

Choose a reason for hiding this comment

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

LGTM. As an aside, do you intend to pull this back out at some point in the future when v4 has been thoroughly tested and older pipelines get migrated, or do you expect this will be necessary permanently (i.e. v3 won't get EOL'd for the foreseeable future)?

app/scripts/modules/core/src/pipeline/create/CreatePipelineModal.tsx
@@ -389,10 +390,7 @@ export class CreatePipelineModal extends React.Component<ICreatePipelineModalPro
</div>
<div className="col-md-7">
<Select
options={[
Copy link
Member

Choose a reason for hiding this comment

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

It's true. Sometimes new Prettier versions disagree with older ones.

@marchello2000
Copy link
Contributor Author

LGTM. As an aside, do you intend to pull this back out at some point in the future when v4 has been thoroughly tested and older pipelines get migrated, or do you expect this will be necessary permanently (i.e. v3 won't get EOL'd for the foreseeable future)?

Good question. There are breaking changes between versions. I believe we can migrate everyone at NFLX but not sure about OSS (likely also true). I, unfortunately, don't have time to do that now, but it's def on my todo list

@marchello2000 marchello2000 added the ready to merge Reviewed and ready for merge label Feb 19, 2020
@mergify mergify bot merged commit 5b90d53 into spinnaker:master Feb 19, 2020
@marchello2000 marchello2000 deleted the mark/default_to_spelv4 branch February 19, 2020 20:46
yunzhangit pushed a commit to yunzhangit/deck that referenced this pull request Mar 28, 2021
@marchello2000 @mergify
feat(spel): default new pipelines to spel v4 (spinnaker#7917)
be87378 
* feat(spel): default new pipelines to spel v4

now that spel v4 is live and glorious in orca let's make deck create new pipelines with that as default

* fixup! feat(spel): default new pipelines to spel v4

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@erikmunson erikmunson erikmunson approved these changes

@christopherthielen christopherthielen Awaiting requested review from christopherthielen

Assignees
No one assigned
Labels
ready to merge Reviewed and ready for merge target-release/1.19
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@marchello2000 @erikmunson @spinnakerbot