Skip to content

libcontainer: prefer bytes.TrimSpace() over strings.TrimSpace()#2609

Merged
AkihiroSuda merged 2 commits into
opencontainers:masterfrom
thaJeztah:byte_trim
Oct 2, 2020
Merged

libcontainer: prefer bytes.TrimSpace() over strings.TrimSpace()#2609
AkihiroSuda merged 2 commits into
opencontainers:masterfrom
thaJeztah:byte_trim

Conversation

@thaJeztah
Copy link
Copy Markdown
Member

@thaJeztah thaJeztah commented Sep 29, 2020

Perform trimming before converting to a string, which should be somewhat more performant.

There may be more cases where we could process variables as byte before converting to strings; see https://medium.com/go-walkthrough/go-walkthrough-bytes-strings-packages-499be9f4b5bd

Also removed some unneeded conversions, caught by my IDE

AkihiroSuda
AkihiroSuda previously approved these changes Sep 29, 2020
View reviewed changes
@kolyshkin
Copy link
Copy Markdown
Contributor

kolyshkin commented Sep 30, 2020

Ahh, we don't have golangci-lint in this repo :(
But that's easy to fix! See #2617, #2618

@thaJeztah
Copy link
Copy Markdown
Member Author

thaJeztah commented Sep 30, 2020

Rebased

@kolyshkin
Copy link
Copy Markdown
Contributor

kolyshkin commented Oct 1, 2020

CI failure is unrelated (temp error from a server)

not ok 53 runc run (hooks library tests)
(from function get_and_extract_debian' in file tests/integration/multi-arch.bash, line 35, from function setup_debian' in file tests/integration/helpers.bash, line 457,
from function setup' in test file tests/integration/hooks.bats, line 16) setup_debian' failed
time="2020-09-30T22:10:14Z" level=fatal msg="Error initializing source docker://amd64/debian:buster: Requesting bear token: invalid status code from registry 500 (Internal Server Error)"

The code that failed is

debian="debian:3.11.6"
skopeo copy docker://amd64/debian:buster "oci:$debian"

If this is repeated frequently, maybe it makes sense to add a retry loop with a sleep.

@thaJeztah
Copy link
Copy Markdown
Member Author

thaJeztah commented Oct 1, 2020

CI is green now 👍

@kolyshkin
Copy link
Copy Markdown
Contributor

kolyshkin commented Oct 1, 2020

@thaJeztah can you please rebase it on top of the current HEAD? I want that shiny new golangci-lint CI job to be run here.

@thaJeztah
libcontainer: prefer bytes.TrimSpace() over strings.TrimSpace()
b3a8b07 
Perform trimming before converting to a string, which should be
somewhat more performant.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@thaJeztah
libcontainer: unconvert
28b452b 
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@thaJeztah
Copy link
Copy Markdown
Member Author

thaJeztah commented Oct 1, 2020

👍 rebased

@kolyshkin
Copy link
Copy Markdown
Contributor

kolyshkin commented Oct 1, 2020

golangci-lint / lint (pull_request) Successful in 24s

Nice ;)

kolyshkin
kolyshkin approved these changes Oct 1, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@kolyshkin kolyshkin left a comment

Choose a reason for hiding this comment

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

LGTM

@AkihiroSuda AkihiroSuda merged commit 750036e into opencontainers:master Oct 2, 2020
@thaJeztah thaJeztah deleted the byte_trim branch October 2, 2020 05:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@kolyshkin kolyshkin kolyshkin approved these changes

@AkihiroSuda AkihiroSuda AkihiroSuda approved these changes

Assignees

No one assigned

Labels

easy-to-review kind/refactor refactoring

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@thaJeztah @kolyshkin @AkihiroSuda