Skip to content

Conversation

@wking
Copy link
Member

@wking wking commented Jul 9, 2018

It's unlikely that base contains single quotes, so this is just a nit. But since %q gives us more robust quoting with fewer source-code characters, it seems like a worthwhile change.

It's unlikely that base contains single quotes, so this is just a nit.
But since %q [1] gives us more robust quoting with fewer source-code
characters, it seems like a worthwhile change.

[1]: https://golang.org/pkg/fmt/#hdr-Printing
@coreosbot
Copy link

Can one of the admins verify this patch?

@openshift-ci-robot openshift-ci-robot added needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Jul 9, 2018
@squat
Copy link
Contributor

squat commented Jul 9, 2018

There may not be quotes but there could be spaces in the path in which case this makes reading the path more obvious.

@squat
Copy link
Contributor

squat commented Jul 9, 2018

/ok-to-test

@openshift-ci-robot openshift-ci-robot removed the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Jul 9, 2018
@squat
Copy link
Contributor

squat commented Jul 9, 2018

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Jul 9, 2018
@wking
Copy link
Member Author

wking commented Jul 9, 2018

There may not be quotes but there could be spaces in the path in which case this makes reading the path more obvious.

The previous code had '%s', which would have handled the spaces-but-no-single-quotes case fine. But the size of this very small corner case doesn't really matter, this PR just addresses the corner cases while also saving a few characters ;).

@wking
Copy link
Member Author

wking commented Jul 9, 2018

The e2e-aws errors look like these.

@stevekuznetsov
Copy link
Contributor

/retest

@wking
Copy link
Member Author

wking commented Jul 9, 2018

Same failures in the most recent run.

@yifan-gu
Copy link
Contributor

/retest

@openshift-merge-robot openshift-merge-robot merged commit 158651d into openshift:master Jul 17, 2018
@wking wking deleted the go-quoted-formatting branch July 17, 2018 01:11
stbenjam pushed a commit to stbenjam/installer that referenced this pull request Feb 10, 2021
Add Owns(Secret, Deployment and ClusterOperator) to be watched
clnperez pushed a commit to clnperez/installer that referenced this pull request Aug 10, 2021
clnperez pushed a commit to clnperez/installer that referenced this pull request Sep 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

lgtm Indicates that a PR is ready to be merged. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants