Skip to content

v1beta1.Trigger delivery should be v1.DeliverySpec#4822

Merged
knative-prow-robot merged 1 commit into
knative:masterfrom
vaikas:ut-for-trigger-filter
Feb 1, 2021
Merged

v1beta1.Trigger delivery should be v1.DeliverySpec#4822
knative-prow-robot merged 1 commit into
knative:masterfrom
vaikas:ut-for-trigger-filter

Conversation

@vaikas
Copy link
Copy Markdown
Contributor

@vaikas vaikas commented Feb 1, 2021

Fixes #

Proposed Changes

  • 🧹 Followup to Fix spec #4515 #4654 and change the trigger.Spec.DeliverySpec from v1beta1 duck to v1. Add UT.

Release Note


Docs

@google-cla google-cla Bot added the cla: yes Indicates the PR's author has signed the CLA. label Feb 1, 2021
@knative-prow-robot knative-prow-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Feb 1, 2021
@knative-metrics-robot
Copy link
Copy Markdown

The following is the coverage report on the affected files.
Say /test pull-knative-eventing-go-coverage to re-run this coverage report

File Old Coverage New Coverage Delta
pkg/apis/eventing/v1/trigger_validation.go 93.2% 96.6% 3.4
pkg/apis/eventing/v1beta1/trigger_conversion.go 93.9% 100.0% 6.1
pkg/apis/eventing/v1beta1/trigger_validation.go 92.6% 96.3% 3.7

@codecov
Copy link
Copy Markdown

codecov Bot commented Feb 1, 2021

Codecov Report

Merging #4822 (b4ea342) into master (ec63881) will increase coverage by 0.11%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #4822      +/-   ##
==========================================
+ Coverage   81.15%   81.27%   +0.11%     
==========================================
  Files         292      292              
  Lines        8311     8309       -2     
==========================================
+ Hits         6745     6753       +8     
+ Misses       1152     1146       -6     
+ Partials      414      410       -4     
Impacted Files Coverage Δ
pkg/apis/eventing/v1beta1/trigger_types.go 100.00% <ø> (ø)
pkg/apis/eventing/v1beta1/trigger_conversion.go 100.00% <100.00%> (+10.81%) ⬆️
pkg/apis/eventing/v1/trigger_validation.go 90.41% <0.00%> (+4.10%) ⬆️
pkg/apis/eventing/v1beta1/trigger_validation.go 89.39% <0.00%> (+4.54%) ⬆️

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 ec63881...026ef43. Read the comment docs.

Copy link
Copy Markdown
Member

@pierDipi pierDipi left a comment

Choose a reason for hiding this comment

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

/lgtm
/approve

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

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: pierDipi, vaikas

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

The pull request process is described here

Details 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

@pierDipi
Copy link
Copy Markdown
Member

pierDipi commented Feb 1, 2021

eventing-gitlab

  Error: go [-e -json -compiled=false -test=false -export=false -deps=true -find=false -- ./...]: exit status 1: go: inconsistent vendoring in /home/runner/work/eventing/eventing/src/knative.dev/eventing-gitlab:
  	knative.dev/eventing: is replaced in go.mod, but not marked as replaced in vendor/modules.txt
  
  run 'go mod vendor' to sync, or use -mod=mod or -mod=readonly to ignore the vendor directory
  
  Usage:
    licenses save <package> [flags]
  
  Flags:
        --force              Delete the destination directory if it already exists.
    -h, --help               help for save
        --save_path string   Directory into which files should be saved that are required by license terms
  
  Global Flags:
        --alsologtostderr                  log to standard error as well as files
        --confidence_threshold float       Minimum confidence required in order to positively identify a license. (default 0.9)
        --log_backtrace_at traceLocation   when logging hits line file:N, emit a stack trace (default :0)
        --log_dir string                   If non-empty, write log files in this directory
        --logtostderr                      log to standard error instead of files
        --stderrthreshold severity         logs at or above this threshold go to stderr (default 2)
    -v, --v Level                          log level for V logs
        --vmodule moduleSpec               comma-separated list of pattern=N settings for file-filtered logging
  
  F0201 11:58:31.937693    4909 main.go:43] go [-e -json -compiled=false -test=false -export=false -deps=true -find=false -- ./...]: exit status 1: go: inconsistent vendoring in /home/runner/work/eventing/eventing/src/knative.dev/eventing-gitlab:
  	knative.dev/eventing: is replaced in go.mod, but not marked as replaced in vendor/modules.txt
  
  run 'go mod vendor' to sync, or use -mod=mod or -mod=readonly to ignore the vendor directory
  --- FAIL: go-licenses failed to update licenses

@vaikas
Copy link
Copy Markdown
Contributor Author

vaikas commented Feb 1, 2021

Thanks @pierDipi I saw that fail before as well and I'm looking at that separately from this. This change by no means should be causing it :)

@vaikas
Copy link
Copy Markdown
Contributor Author

vaikas commented Feb 1, 2021

Comment thread pkg/apis/eventing/v1beta1/trigger_types.go
@slinkydeveloper
Copy link
Copy Markdown
Contributor

/hold

@knative-prow-robot knative-prow-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Feb 1, 2021
@slinkydeveloper
Copy link
Copy Markdown
Contributor

/unhold

@knative-prow-robot knative-prow-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Feb 1, 2021
@knative-prow-robot knative-prow-robot merged commit 012a9bd into knative:master Feb 1, 2021
@vaikas vaikas deleted the ut-for-trigger-filter branch February 1, 2021 15:38
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. 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.

5 participants