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

zapslog: Drop HandlerOptions.New, use NewHandler #1315

Merged
merged 1 commit into from
Aug 3, 2023

Conversation

abhinav
Copy link
Collaborator

@abhinav abhinav commented Aug 3, 2023

The pattern of constructors for zapslog.Handler was previously:

type HandlerOptions struct{ ... }

func (*HandlerOptions) New(zapcore.Core) *Handler
func NewHandler(zapcore.Core) *Handler

This was modeled after similar constructors in slog for JSON and Text
handlers.

slog has since dropped those constructors in favor of simple:

func NewJSONHandler(io.Writer, *HandlerOptions) *JSONHandler
func NewTextHandler(io.Writer, *HandlerOptions) *TextHandler

This change similarly drops the options.New method and default
no-argument constructor in favor of:

func NewHandler(zapcore.Core, *HandlerOptions) *Handler

As with slog's JSON or Text handlers, the first argument is the
destination: an io.Writer for JSON and Text, a Zap core for us.

Refs golang/go#59339

The pattern of constructors for zapslog.Handler was previously:

    type HandlerOptions struct{ ... }

    func (*HandlerOptions) New(zapcore.Core) *Handler
    func NewHandler(zapcore.Core) *Handler

This was modeled after similar constructors in slog for JSON and Text
handlers.

slog has since dropped those constructors in favor of simple:

    func NewJSONHandler(io.Writer, *HandlerOptions) *JSONHandler
    func NewTextHandler(io.Writer, *HandlerOptions) *TextHandler

This change similarly drops the options.New method and default
no-argument constructor in favor of:

    func NewHandler(zapcore.Core, *HandlerOptions) *Handler

As with slog's JSON or Text handlers, the first argument is the
destination: an io.Writer for JSON and Text, a Zap core for us.

Refs golang/go#59339
@abhinav abhinav requested a review from sywhang August 3, 2023 15:38
@abhinav
Copy link
Collaborator Author

abhinav commented Aug 3, 2023

[2023-08-03T15:39:04.953Z] ['error'] There was an error running the uploader: Error uploading to [https://codecov.io:](https://codecov.io/) Error: There was an error fetching the storage URL during POST: 404 - {'detail': ErrorDetail(string='Unable to locate build via Github Actions API. Please upload with the Codecov repository upload token to resolve issue.', code='not_found')}

codecov upload seems to be having trouble

@codecov
Copy link

codecov bot commented Aug 3, 2023

Codecov Report

Merging #1315 (97ce357) into master (7d42ce9) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #1315   +/-   ##
=======================================
  Coverage   97.93%   97.93%           
=======================================
  Files          50       50           
  Lines        3249     3249           
=======================================
  Hits         3182     3182           
  Misses         57       57           
  Partials       10       10           

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@abhinav abhinav merged commit 9c3c581 into uber-go:master Aug 3, 2023
4 checks passed
@abhinav abhinav deleted the slog-handler branch August 3, 2023 16:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants