Skip to content

Complete the migration of the Revision controller to level-based.#1334

Merged
google-prow-robot merged 3 commits intoknative:masterfrom
mattmoor:fluentd-configmap
Jun 26, 2018
Merged

Complete the migration of the Revision controller to level-based.#1334
google-prow-robot merged 3 commits intoknative:masterfrom
mattmoor:fluentd-configmap

Conversation

@mattmoor
Copy link
Copy Markdown
Member

This moves the remaining fluentd ConfigMap and autoscaler reconciliations to follow the same pattern as the rest of the Revision controller.

Fixes: #1217

WIP until #1328 is merged and this rebased. I may also try and improve the coverage of the autoscaler parts, which are in need of some TLC.

@google-prow-robot google-prow-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Jun 24, 2018
@mattmoor mattmoor changed the title Complete the migration of the Revision controller to level-based. [WIP] Complete the migration of the Revision controller to level-based. Jun 24, 2018
@google-prow-robot google-prow-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jun 24, 2018
@mattmoor mattmoor force-pushed the fluentd-configmap branch 2 times, most recently from 9840072 to 2be1073 Compare June 26, 2018 01:37
@google-prow-robot google-prow-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Jun 26, 2018
@mattmoor mattmoor changed the title [WIP] Complete the migration of the Revision controller to level-based. Complete the migration of the Revision controller to level-based. Jun 26, 2018
@google-prow-robot google-prow-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jun 26, 2018
@mattmoor
Copy link
Copy Markdown
Member Author

/assign @vaikas-google
/assign @dprotaso

cc @josephburnett

@mattmoor
Copy link
Copy Markdown
Member Author

FYI I believe the coverage discrepancy is an exacerbation of #1331

@dprotaso
Copy link
Copy Markdown
Member

did a quick pass
/lgtm

@google-prow-robot google-prow-robot added the lgtm Indicates that a PR is ready to be merged. label Jun 26, 2018
@mattmoor mattmoor force-pushed the fluentd-configmap branch from 8d1bcf9 to 30f844d Compare June 26, 2018 19:32
@google-prow-robot google-prow-robot removed the lgtm Indicates that a PR is ready to be merged. label Jun 26, 2018
@knative-metrics-robot
Copy link
Copy Markdown

The following is the coverage report on pkg/. Say /test pull-knative-serving-go-coverage to run the coverage report again

File Old Coverage New Coverage Delta
pkg/controller/revision/revision.go 78.8% 72.1% -6.7

*TestCoverage feature is being tested, do not rely on any info here yet

@vaikas
Copy link
Copy Markdown
Contributor

vaikas commented Jun 26, 2018

/lgtm
/approve

@google-prow-robot google-prow-robot added the lgtm Indicates that a PR is ready to be merged. label Jun 26, 2018
@google-prow-robot
Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mattmoor, vaikas-google

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

@google-prow-robot google-prow-robot merged commit a600c19 into knative:master Jun 26, 2018
@mattmoor mattmoor deleted the fluentd-configmap branch June 26, 2018 20:04
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. lgtm Indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants