Skip to content

Conversation

@yolocs
Copy link
Contributor

@yolocs yolocs commented Jan 14, 2020

Fixes knative/eventing#1987
Eventing PR: knative/eventing#2382

Proposed Changes

@googlebot googlebot added the cla: yes Indicates the PR's author has signed the CLA. label Jan 14, 2020
@knative-prow-robot knative-prow-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Jan 14, 2020
- https://github.com/knative/serving/releases/download/{{< version >}}/monitoring-tracing-zipkin.yaml
- https://github.com/knative/serving/releases/download/{{< version >}}/monitoring-tracing-zipkin-in-mem.yaml
- **Eventing Component**:
- https://github.com/knative/eventing/releases/download/{{< version >}}/release.yaml
Copy link
Contributor

Choose a reason for hiding this comment

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

Just to make sure I understand, @yolocs -- going forward, the only artifact released for eventing will be eventing.yaml, and it will come bundled with the in-memory channel provisioner? No more option to install it separately?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, that's my understanding. @grantr

Copy link
Contributor

Choose a reason for hiding this comment

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

Cool, just wanted to make sure I understood what was changing :) I'll wait to merge this until the Eventing PR is merged.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

FYI. The eventing PR has been merged.

Copy link
Contributor

Choose a reason for hiding this comment

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

the only artifact released for eventing will be eventing.yaml, and it will come bundled with the in-memory channel provisioner? No more option to install it separately?

No, this is incorrect. There will still be 3 artifacts:

  • eventing-core.yaml will be just the eventing core without InMemoryChannel, equivalent to the previous eventing.yaml
    in-memory-channel.yaml will be just the InMemoryChannel, unchanged
  • eventing.yaml will be eventing-core.yaml and in-memory-channel.yaml combined, equivalent to the previous release.yaml

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If I understand it correctly, only eventing.yaml will actually be useful, right? I don't see serving list serving-core.yaml here.

Copy link
Contributor

Choose a reason for hiding this comment

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

You're right, serving-core.yaml isn't documented here so eventing-core.yaml can also be undocumented. I was just answering @samodell's question about installing separately. That option will still exist, even if undocumented.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for clarifying.

@samodell
Copy link
Contributor

/lgtm
/approve

@knative-prow-robot knative-prow-robot added the lgtm Indicates that a PR is ready to be merged. label Jan 15, 2020
@knative-prow-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: samodell, yolocs

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@knative-prow-robot knative-prow-robot merged commit bbc946d into knative:master Jan 15, 2020
@yolocs yolocs deleted the b/fix-artifacts-names branch January 15, 2020 21:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla: yes Indicates the PR's author has signed the CLA. lgtm Indicates that a PR is ready to be merged. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Eventing release file names aren't consistent with serving

5 participants