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

Update for staticcheck compliance #119

Merged
merged 17 commits into from
Mar 21, 2024
Merged

Conversation

geofffranks
Copy link
Contributor

👋Howdy!

We (the App Platform Runtime WG) pull credhub-cli into the diego-release codebase, and are trying to run staticcheck on all of its codebases. We found a few things flagged on credhub-cli, and would love it if y'all could incorporate these changes so we don't have to jump through hoops to whitelist certain codebases from our staticcheck automation.

@geofffranks geofffranks marked this pull request as ready for review March 6, 2024 14:04
@peterhaochen47
Copy link
Member

peterhaochen47 commented Mar 7, 2024

Hi, thank you! We'll take a look!

But meanwhile, there are 2 test failures from running make all on this branch:

[1709773266] CredHub API Client Suite - 182/182 specs - 15 procs •••••••••••••••••••••••••••••••••••••••••••••••••••••••••••••••••••••••
------------------------------
• [FAILED] [0.001 seconds]
Client() When a SOCKS5Proxy is set in the environment [It] returns an http.Client with a Dial function
/Users/peterch/workspace/tmp/credhub-cli/credhub/client_test.go:26

  [FAILED] Expected
      <func(context.Context, string, string) (net.Conn, error)>: nil
  not to be nil
  In [It] at: /Users/peterch/workspace/tmp/credhub-cli/credhub/client_test.go:33 @ 03/06/24 17:01:24.452
------------------------------
•••••••••••••••••••••••
------------------------------
• [FAILED] [0.002 seconds]
Client() With Dial [It] should return a http.Client with a dial function
/Users/peterch/workspace/tmp/credhub-cli/credhub/client_test.go:140

  [FAILED] Expected
      <func(context.Context, string, string) (net.Conn, error)>: nil
  not to be nil
  In [It] at: /Users/peterch/workspace/tmp/credhub-cli/credhub/client_test.go:146 @ 03/06/24 17:01:24.457
------------------------------
••••••••••••••••••••••••••••••••••••••••••••••••••••••••••••••••••••••••••••••••••••••

Summarizing 2 Failures:
  [FAIL] Client() With Dial [It] should return a http.Client with a dial function
  /Users/peterch/workspace/tmp/credhub-cli/credhub/client_test.go:146
  [FAIL] Client() When a SOCKS5Proxy is set in the environment [It] returns an http.Client with a Dial function
  /Users/peterch/workspace/tmp/credhub-cli/credhub/client_test.go:33

Ran 182 of 182 Specs in 0.064 seconds
FAIL! -- 180 Passed | 2 Failed | 0 Pending | 0 Skipped

@Tallicia
Copy link
Contributor

Hi @geofffranks Any update on these failures?

@geofffranks
Copy link
Contributor Author

oh, thanks! i missed this comment. Will take a look today.

@geofffranks
Copy link
Contributor Author

Ok, should pass tests now @Tallicia @peterhaochen47 - thanks!

@Tallicia
Copy link
Contributor

All tests passed on local confirmation. Planning for merge.
Screenshot 2024-03-21 at 1 20 59 PM

Copy link
Contributor

@Tallicia Tallicia left a comment

Choose a reason for hiding this comment

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

Updates for staticCheck look ok

@Tallicia Tallicia merged commit 6b3dc17 into cloudfoundry:main Mar 21, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging this pull request may close these issues.

None yet

3 participants