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

Replace 200, HEAD and OPTIONS with constants from net/http #243

Merged
merged 1 commit into from Aug 16, 2023

Conversation

iBug
Copy link
Contributor

@iBug iBug commented Aug 6, 2023

What type of PR is this? (check all applicable)

  • Refactor
  • Feature
  • Bug Fix
  • Optimization
  • Documentation Update

Description

A previous commit (#241) replaced most HTTP constants with those from standard library net/http, but some was missed. This PR catches 3 of them.

Related Tickets & Documents

  • Related Issue: None
  • Closes: None

Added/updated tests?

  • Yes
  • No, and this is why: Trivial changes, tests not required
  • I need help with writing tests

Run verifications and test

  • make verify is passing (which I believe is not my fault)
  • make test is passing

@coreydaley coreydaley self-assigned this Aug 7, 2023
@codecov
Copy link

codecov bot commented Aug 7, 2023

Codecov Report

Merging #243 (295a81b) into main (0eda2fc) will not change coverage.
Report is 1 commits behind head on main.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main     #243   +/-   ##
=======================================
  Coverage   84.44%   84.44%           
=======================================
  Files           7        7           
  Lines         598      598           
=======================================
  Hits          505      505           
  Misses         82       82           
  Partials       11       11           
Files Changed Coverage Δ
cors.go 96.02% <ø> (ø)

@coreydaley
Copy link
Contributor

@iBug Would you mind posting the results of the make verify that is not passing for you? Along with what Operating System and Go version that you are using?

@coreydaley coreydaley enabled auto-merge (squash) August 7, 2023 02:12
@iBug
Copy link
Contributor Author

iBug commented Aug 7, 2023

@coreydaley Ubuntu 22.04 LTS, Go 1.20.7.

With one exception (first one below), all errors are from files under /usr/local/go/src where my Go installation is located.

canonical_test.go:55:33: missing type in composite literal (typecheck)
        querystring := url.Values{"q": {"golang"}, "format": {"json"}}.Encode()
                                       ^
../../../../usr/local/go/src/runtime/debuglog.go:296:20: StringData not declared by package unsafe (typecheck)
        strData := unsafe.StringData(x)
                          ^
../../../../usr/local/go/src/runtime/heapdump.go:159:37: StringData not declared by package unsafe (typecheck)
        dumpmemrange(unsafe.Pointer(unsafe.StringData(s)), uintptr(len(s)))
                                           ^
../../../../usr/local/go/src/runtime/heapdump.go:202:32: StringData not declared by package unsafe (typecheck)
                dwrite(unsafe.Pointer(unsafe.StringData(pkgpath)), uintptr(len(pkgpath)))

However, if I do make verify inside a Docker container (image golang:1.20-bookworm), then everything's fine. I guess I can tick that checkbox now.

@coreydaley coreydaley merged commit fc93057 into gorilla:main Aug 16, 2023
10 checks passed
@iBug iBug deleted the typos branch August 21, 2023 16:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: ✅ Done
Development

Successfully merging this pull request may close these issues.

None yet

2 participants