Skip to content

features: rename features to FeatureGate #247

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
Merged
derekwaynecarr merged 1 commit into from
Mar 13, 2019
Merged

features: rename features to FeatureGate #247

derekwaynecarr merged 1 commit into from
Mar 13, 2019

Conversation

@rphillips
Copy link
Contributor

@rphillips rphillips commented Mar 12, 2019 •
edited
Loading

I was getting this error on my informer. Note FeatureList does not exist, and FeaturesList does. We need to redo the pluralization and update the go-client.

I0305 14:38:13.242898   22751 reflector.go:169] Listing and watching *v1.Features from github.com/openshift/client-go/config/informers/externalversions/factory.go:101
E0305 14:38:13.244280   22751 reflector.go:134] github.com/openshift/client-go/config/informers/externalversions/factory.go:101: Failed to list *v1.Features: no kind "FeatureList" is registered for version "config.openshift.io/v1" in scheme "github.com/openshift/client-go/c
onfig/clientset/versioned/scheme/register.go:14"

Reopens: #234
/cc @sjenning @derekwaynecarr @deads2k

@openshift-ci-robot openshift-ci-robot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Mar 12, 2019
@derekwaynecarr
Copy link
Member

derekwaynecarr commented Mar 12, 2019

lets rename Feature -> FeatureSet
lets rename FeatureSet -> FeatureSetName

@derekwaynecarr
Copy link
Member

derekwaynecarr commented Mar 12, 2019

hmm, naming is hard, wondering if *Set will confuse with replica set or statefulset

@derekwaynecarr
Copy link
Member

derekwaynecarr commented Mar 12, 2019

how about Feature -> FeatureGate
and FeatureSet stays put.

@sjenning
Copy link
Contributor

sjenning commented Mar 12, 2019

fyi PRs that rename types will not pass verify-client-go

@rphillips rphillips force-pushed the fixes/fix_pluralization_feature branch from 5a9b1a3 to ee96191 Compare March 12, 2019 22:45
@openshift-ci-robot openshift-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Mar 12, 2019
@openshift-ci-robot
Copy link

openshift-ci-robot commented Mar 12, 2019

@rphillips: The following test failed, say /retest to rerun them all:

Test name Commit Details Rerun command
ci/prow/verify-client-go ee96191 link /test verify-client-go

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@rphillips rphillips changed the title features: client generator expects singular objects features: rename features to FeatureGate Mar 12, 2019
@rphillips
Copy link
Contributor Author

rphillips commented Mar 12, 2019

@sjenning @derekwaynecarr pushed the rename to this PR

@derekwaynecarr
Copy link
Member

derekwaynecarr commented Mar 13, 2019

force merging.

@sjenning
Copy link
Contributor

sjenning commented Mar 13, 2019

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Mar 13, 2019
@openshift-ci-robot
Copy link

openshift-ci-robot commented Mar 13, 2019

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: rphillips, sjenning

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

@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 13, 2019
@derekwaynecarr derekwaynecarr merged commit 5e45fff into openshift:master Mar 13, 2019
@adambkaplan
Copy link
Contributor

adambkaplan commented Mar 18, 2019

@derekwaynecarr this PR blocks bumps on origin:

# github.com/openshift/origin/pkg/admission/customresourcevalidation/features
pkg/admission/customresourcevalidation/features/validate_features.go:34:46: undefined: "github.com/openshift/origin/vendor/github.com/openshift/api/config/v1".Features
pkg/admission/customresourcevalidation/features/validate_features.go:41:25: undefined: "github.com/openshift/origin/vendor/github.com/openshift/api/config/v1".Features
pkg/admission/customresourcevalidation/features/validate_features.go:56:37: undefined: "github.com/openshift/origin/vendor/github.com/openshift/api/config/v1".FeaturesSpec
pkg/admission/customresourcevalidation/features/validate_features.go:67:46: undefined: "github.com/openshift/origin/vendor/github.com/openshift/api/config/v1".FeaturesSpec

@sjenning sjenning mentioned this pull request Mar 21, 2019
Merged
@sjenning
Copy link
Contributor

sjenning commented Mar 21, 2019

@adambkaplan sorry for letting this sit around. openshift/origin#22389 should fix.

wking added a commit to wking/openshift-installer that referenced this pull request Apr 1, 2019
@wking
vendor: Bump openshift/api to pick up InfrastructureName
3208111 
Catching up with openshift/api@c9b4e5b2d7 (Add InfrastructureName,
2019-03-01, openshift/api#266).

Generated with:

  $ dep ensure -update github.com/openshift/api
  $ dep ensure -update github.com/openshift/client-go

using:

  $ dep version
  dep:
   version     : v0.5.1
   build date  : 2019-03-20
   git hash    : faa61893
   go version  : go1.10.3
   go compiler : gc
   platform    : linux/amd64
   features    : ImportDuringSolve=false

The client-go bump avoids:

  vendor/github.com/openshift/client-go/config/clientset/versioned/typed/config/v1/features.go:22:25: undefined: "github.com/openshift/installer/vendor/github.com/openshift/api/config/v1".Features

by pulling in openshift/client-go@035cd9bc (bump API and regenerate
for FeatureGate API, 2019-03-13, openshift/client-go#97) to match
openshift/api@ee961910 (features: rename features to FeatureGate,
2019-03-05, openshift/api#247), which was also pulled in by the API
bump.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@abhinavdahiya abhinavdahiya Awaiting requested review from abhinavdahiya

@adambkaplan adambkaplan Awaiting requested review from adambkaplan

Assignees

@sjenning sjenning

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/M Denotes a PR that changes 30-99 lines, ignoring generated files.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@rphillips @derekwaynecarr @sjenning @openshift-ci-robot @adambkaplan