Skip to content
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.

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

Improve empty string test #473

Merged
merged 1 commit into from
Mar 31, 2021
Merged

Conversation

rayandas
Copy link
Contributor

Signed-off-by: Rayan Das rayandas91@gmail.com

Description

A string can be tested for its emptiness either by treating it as a slice and calculating the length of the slice, or by treating it as a string and directly comparing the value. While both produce identical code when compiled, it makes more sense to treat a string as itself, than a slice, for the sake of comparison of values.

For example:

len(xyz) == 0

can be rewritten as:

xyz == ""

Why is this needed

The second method is considered more idiomatic.

@codecov
Copy link

codecov bot commented Mar 31, 2021

Codecov Report

Merging #473 (b5064e9) into master (c25ee26) will not change coverage.
The diff coverage is 100.00%.

❗ Current head b5064e9 differs from pull request most recent head 1dd406a. Consider uploading reports for the commit 1dd406a to get more accurate results
Impacted file tree graph

@@           Coverage Diff           @@
##           master     #473   +/-   ##
=======================================
  Coverage   32.32%   32.32%           
=======================================
  Files          50       50           
  Lines        3276     3276           
=======================================
  Hits         1059     1059           
  Misses       2123     2123           
  Partials       94       94           
Impacted Files Coverage Δ
grpc-server/tinkerbell.go 92.12% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c25ee26...1dd406a. Read the comment docs.

@gianarb gianarb requested a review from gauravgahlot March 31, 2021 08:00
@gauravgahlot
Copy link
Contributor

@rayandas Could you please update the branch?

Signed-off-by: Rayan Das <rayandas91@gmail.com>
@rayandas
Copy link
Contributor Author

@gauravgahlot updated.

@gauravgahlot gauravgahlot added the ready-to-merge Signal to Mergify to merge the PR. label Mar 31, 2021
@mergify mergify bot merged commit 838453e into tinkerbell:master Mar 31, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-to-merge Signal to Mergify to merge the PR.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants