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

Simple refactors #283

Merged
merged 4 commits into from
Sep 9, 2020
Merged

Simple refactors #283

merged 4 commits into from
Sep 9, 2020

Conversation

mmlb
Copy link
Contributor

@mmlb mmlb commented Sep 8, 2020

Description

This PR cleans up some code that stood out as pretty weird when I was reading through it. Some simple function calls were being wrapped in a local var and then called via it. This is pretty odd since we can just as easily call the underlying function. Along the way I cleaned up the msg variable definitions from being done in multiple lines into just one as a const to better reflect how its actually used.

How Has This Been Tested?

Still compiles, tests pass.

@codecov
Copy link

codecov bot commented Sep 8, 2020

Codecov Report

Merging #283 into master will increase coverage by 0.20%.
The diff coverage is 6.45%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #283      +/-   ##
==========================================
+ Coverage   20.58%   20.78%   +0.20%     
==========================================
  Files          15       15              
  Lines        1365     1342      -23     
==========================================
- Hits          281      279       -2     
+ Misses       1070     1049      -21     
  Partials       14       14              
Impacted Files Coverage Δ
grpc-server/hardware.go 0.00% <0.00%> (ø)
grpc-server/workflow.go 0.00% <0.00%> (ø)
http-server/http_handlers.go 7.08% <ø> (ø)
grpc-server/template.go 19.23% <25.00%> (-0.42%) ⬇️

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 547e8ea...0b0de6b. Read the comment docs.

@mmlb mmlb mentioned this pull request Sep 8, 2020
3 tasks
@mmlb mmlb requested review from kqdeng, parauliya and gauravgahlot and removed request for kqdeng September 8, 2020 23:23
gauravgahlot
gauravgahlot previously approved these changes Sep 9, 2020
gianarb
gianarb previously approved these changes Sep 9, 2020
mmlb added 4 commits September 9, 2020 09:31
Signed-off-by: Manuel Mendez <[email protected]>
So the pb.go files are in line with our coding standards.

Signed-off-by: Manuel Mendez <[email protected]>
These are never re-assigned to so there's no real reason to define as an
empty string and later updated a couple lines below. After that clean up
there's no reason not to make it a `const`. Having it `const` helps to
convey that its never meant to be updated.

Signed-off-by: Manuel Mendez <[email protected]>
`fn` is only ever assigned to once and then just called. This is no
different than just calling the underlying functions, so lets do that.

Signed-off-by: Manuel Mendez <[email protected]>
@mmlb mmlb dismissed stale reviews from gianarb and gauravgahlot via 0b0de6b September 9, 2020 13:35
@mmlb mmlb requested review from gianarb and gauravgahlot and removed request for kqdeng and parauliya September 9, 2020 14:17
@mmlb mmlb added the ready-to-merge Signal to Mergify to merge the PR. label Sep 9, 2020
@mmlb mmlb removed the request for review from gianarb September 9, 2020 14:39
@mergify mergify bot merged commit 87e74b9 into tinkerbell:master Sep 9, 2020
@mmlb mmlb deleted the simple-refactors branch September 9, 2020 14:44
@mmlb mmlb removed the ready-to-merge Signal to Mergify to merge the PR. label Jan 25, 2021
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