Skip to content

feat: add allow auto update schema cmd#567

Merged
nodece merged 1 commit intostreamnative:masterfrom
nodece:ns_policies_auto_update_schema
Jan 24, 2022
Merged

feat: add allow auto update schema cmd#567
nodece merged 1 commit intostreamnative:masterfrom
nodece:ns_policies_auto_update_schema

Conversation

@nodece
Copy link
Copy Markdown
Contributor

@nodece nodece commented Jan 24, 2022

Signed-off-by: Zixuan Liu nodeces@gmail.com

@nodece nodece requested review from a team and zymap as code owners January 24, 2022 08:42
@nodece nodece force-pushed the ns_policies_auto_update_schema branch 2 times, most recently from 26319a6 to fd28ca2 Compare January 24, 2022 08:50
Copy link
Copy Markdown
Member

@zymap zymap left a comment

Choose a reason for hiding this comment

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

LGTM


var examples []cmdutils.Example
examples = append(examples, cmdutils.Example{
Desc: "Get allow auto update update schema on a namespace",
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
Desc: "Get allow auto update update schema on a namespace",
Desc: "Get the whether to allow auto update schema on a namespace",

Comment on lines +59 to +63
if result != nil {
vc.Command.Println(strconv.FormatBool(*result))
} else {
vc.Command.Println(result)
}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
if result != nil {
vc.Command.Println(strconv.FormatBool(*result))
} else {
vc.Command.Println(result)
}
vc.Command.Println(strconv.FormatBool(*result))

Looks like printing the result directly is enough

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

We need to check nil pointer.

Comment thread pkg/ctl/namespace/set_is_allow_auto_update_schema.go Outdated
Comment thread pkg/ctl/namespace/set_is_allow_auto_update_schema.go Outdated
Comment thread pkg/ctl/namespace/set_is_allow_auto_update_schema.go Outdated
Comment thread pkg/ctl/namespace/set_is_allow_auto_update_schema.go Outdated
@nodece nodece force-pushed the ns_policies_auto_update_schema branch 3 times, most recently from 6a96a9e to 592207c Compare January 24, 2022 10:39
Signed-off-by: Zixuan Liu <nodeces@gmail.com>
@nodece nodece force-pushed the ns_policies_auto_update_schema branch from 592207c to d1bb3ac Compare January 24, 2022 11:59
@nodece nodece merged commit ed7f745 into streamnative:master Jan 24, 2022
tisonkun pushed a commit to tisonkun/pulsar-client-go that referenced this pull request Aug 15, 2023
Signed-off-by: Zixuan Liu <nodeces@gmail.com>
tisonkun pushed a commit to apache/pulsar-client-go that referenced this pull request Aug 16, 2023
Signed-off-by: Zixuan Liu <nodeces@gmail.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.

2 participants