Skip to content

Conversation

fanminshi
Copy link
Contributor

@fanminshi fanminshi commented Feb 22, 2018

allows command new to generate all necessary files under pkg/apis/**.
and refactor the code in renderPkg() to make it more readable.
change naming apis -> api

@fanminshi
Copy link
Contributor Author

Manual Test:
$ ./operator-sdk/operator-sdk new play-operator --api-version=play.example.com/v1alpha1 --kind PlayService
Output:

└── play-operator
    ├── cmd
    │   └── play-operator
    │       └── main.go
    ├── config
    ├── deploy
    ├── pkg
    │   ├── apis
    │   │   └── play
    │   │       └── v1alpha1
    │   │           ├── doc.go
    │   │           ├── register.go
    │   │           └── types.go
    │   └── stub
    │       └── handler.go
    └── tmp
        ├── build
        └── codegen

@fanminshi
Copy link
Contributor Author

cc/ @hasbro17 @hongchaodeng

return ioutil.WriteFile(filepath.Join(apiDir, types), buf.Bytes(), 0644)
}

func renderSubDirFiles(stubDir string) error {
Copy link
Contributor

Choose a reason for hiding this comment

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

SubDir is confusing. Can you rename it better?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sorry my mistake. it should be stubDir

return err
}
if err := ioutil.WriteFile(filepath.Join(sDir, handler), buf.Bytes(), 0644); err != nil {
if err := ioutil.WriteFile(filepath.Join(apiDir, doc), buf.Bytes(), 0644); err != nil {
Copy link
Contributor

Choose a reason for hiding this comment

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

0644

define const

if err := os.MkdirAll(filepath.Join(g.projectName, apisDir, apiDirName(g.apiVersion), version(g.apiVersion)), defaultFileMode); err != nil {
v := version(g.apiVersion)
apiDir := filepath.Join(g.projectName, apisDir, apiDirName(g.apiVersion), v)
if err := os.MkdirAll(apiDir, defaultFileMode); err != nil {
Copy link
Contributor

Choose a reason for hiding this comment

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

defaultFileMode -> defaultFileModeForFolder

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think defaultDirFileMode is more concise.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sounds good

return nil

buf = &bytes.Buffer{}
if err := renderApisRegister(buf, kind, groupName, version); err != nil {
Copy link
Contributor

Choose a reason for hiding this comment

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

renderAPIDirRegisterFile

Or renderAPIRegisterFile. But change renderAPIDirFiles -> renderAPIFiles

Copy link
Contributor Author

Choose a reason for hiding this comment

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

kk, I am going to follow this convention:
if rendering a dir such as renderPkg, no need to add Dir suffix.
If rendering a file such as renderApisRegister, then we want to be more specific as in renderAPIRegisterFile

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will have another pr to refactor other render* functions to follow above convention.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sounds good

@fanminshi
Copy link
Contributor Author

Latest test output:

play-operator/
├── cmd
│   └── play-operator
│       └── main.go
├── config
├── deploy
├── pkg
│   ├── apis
│   │   └── play
│   │       └── v1alpha1
│   │           ├── doc.go
│   │           ├── register.go
│   │           └── types.go
│   └── stub
│       └── handler.go
└── tmp
    ├── build
    └── codegen

@fanminshi
Copy link
Contributor Author

all fixed! PTAL cc/ @hongchaodeng

@hongchaodeng
Copy link
Contributor

LGTM

@fanminshi fanminshi merged commit b1e731e into operator-framework:master Feb 22, 2018
@hongchaodeng hongchaodeng deleted the hook_gen_apis branch February 22, 2018 21:21
)

// Doc contains all the customized data needed to generate apis/../doc.go for a new operator
// Doc contains all the customized data needed to generate apis/<version>/doc.go for a new operator
Copy link
Contributor

Choose a reason for hiding this comment

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

Just curious but is the comment supposed to be apis/<apiDirName>/<version>/doc.go since the example generated file is at pkg/apis/play/v1alpha1/doc.go.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@hasbro17 good catch! will fix this in a new pr.

jmrodri added a commit that referenced this pull request Feb 17, 2022
Additions:

- chore: bump go 1.17, k8s 1.23, and kubebuilder 3.3 (#69)
- chore(deps): update to Quarkus SDK extension 3.0.2 and Quarkus 2.6.3 (#70)
- chore(deps): update to use Quarkus JOSDK extension 3.0.1 (#67)
- Remove useless file (#65)
- chore: bump k8s 1.22.2 and kubebuilder 3.2 (#64)
- exposed exnpoints for micrometer metrics (#45)
- modified the Quarkus operator SDK version and tutorial too (#40)
- chore: bump fabric8 5.8.0 & quarkus 2.4.0 (#42)
- modified the doc file and removed file after generation (#41)
- release: fix release script to understand release branches (#38)

Bug Fixes:

- Fix for wrongly generated file name (#73)

Signed-off-by: jesus m. rodriguez <jesusr@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants