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

Move eventing-contrib/cmd/* here #5295

Merged
merged 4 commits into from
Apr 22, 2021

Conversation

vaikas
Copy link
Contributor

@vaikas vaikas commented Apr 21, 2021

The eventing-contrib/cmd tools were left around after the various other pieces moved to knative-sandbox/* repos. There have been no releases for those since Dec 2020. As agreed, we decided to move them into the knative/eventing repo so that we'll get proper updates / releases, etc.

Couple of notes:

  • I did not update the copyright years, but instead kept them as is since they were moved from our own repo to here.
  • I looked around for seeing if there was an easy way to release them into the eventing-contrib registry, but could not find one. This was discussed on slack and I agreed to take a look into it to see if it's possible, doesn't look like it. Plus I actually think it's cleaner if from the image you can easily see where it came from.

Proposed Changes

Pre-review Checklist

These are simple tools used for our docs examples that are simply moved here. I'll create the docs PR once I verify the release goes (I'll test with nightly) out correctly.

  • At least 80% unit test coverage
  • E2E tests for any new behavior
  • Docs PR for any user-facing impact
  • Spec PR for any new API feature
  • Conformance test for any change to the spec

Release Note

Tools from https://github.com/knative/eventing-contrib/tree/main/cmd were moved to eventing repo.

Docs
knative/docs#3489

@google-cla google-cla bot added the cla: yes Indicates the PR's author has signed the CLA. label Apr 21, 2021
@knative-prow-robot knative-prow-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. area/test-and-release Test infrastructure, tests or release labels Apr 21, 2021
@codecov
Copy link

codecov bot commented Apr 21, 2021

Codecov Report

Merging #5295 (6c82a3d) into main (1f54d0e) will increase coverage by 0.30%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #5295      +/-   ##
==========================================
+ Coverage   83.34%   83.64%   +0.30%     
==========================================
  Files         243      243              
  Lines        6879     6896      +17     
==========================================
+ Hits         5733     5768      +35     
+ Misses        797      785      -12     
+ Partials      349      343       -6     
Impacted Files Coverage Δ
pkg/apis/messaging/v1/subscription_validation.go 80.00% <0.00%> (-1.40%) ⬇️
pkg/reconciler/broker/trigger/trigger.go 80.45% <0.00%> (+0.45%) ⬆️
pkg/reconciler/channel/channel.go 75.40% <0.00%> (+3.27%) ⬆️
pkg/broker/ingress/ingress_handler.go 75.75% <0.00%> (+12.99%) ⬆️
pkg/eventfilter/attributes/filter.go 100.00% <0.00%> (+13.63%) ⬆️
pkg/apis/messaging/v1/subscription_defaults.go 100.00% <0.00%> (+66.66%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1f54d0e...6c82a3d. Read the comment docs.

@vaikas
Copy link
Contributor Author

vaikas commented Apr 21, 2021

Lint fails with:

  Error: SA1019: cloudevents.NewDefaultClient is deprecated: Please use NewClientHTTP with the observability options.  (staticcheck)
  Error: SA1019: cloudevents.NewDefaultClient is deprecated: Please use NewClientHTTP with the observability options.  (staticcheck)
  Error: SA1019: cloudevents.NewDefaultClient is deprecated: Please use NewClientHTTP with the observability options.  (staticcheck)

Copy link
Member

@matzew matzew left a comment

Choose a reason for hiding this comment

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

tested the event-display with the png source:

k logs -f -l serving.knative.dev/service=event-display -c user-container                        
  specversion: 1.0
  type: dev.knative.sources.ping
  source: /apis/v1/namespaces/default/pingsources/ping-source
  id: 93439c6d-0f05-45b3-85c2-78ca77cd64a6
  time: 2021-04-22T06:57:00.20280561Z
  datacontenttype: application/json
Data,
  {
    "message": "Hello world!"
  }

/lgtm
/approve

@knative-prow-robot knative-prow-robot added the lgtm Indicates that a PR is ready to be merged. label Apr 22, 2021
@knative-prow-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: matzew

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 added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 22, 2021
@knative-prow-robot knative-prow-robot merged commit 7775e37 into knative:main Apr 22, 2021
@vaikas vaikas deleted the eventing-contrib-cmd-here branch April 22, 2021 15:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. area/test-and-release Test infrastructure, tests or release cla: yes Indicates the PR's author has signed the CLA. lgtm Indicates that a PR is ready to be merged. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants