feat: add http.StartServer2 with more customization #725
Conversation
|
@jrschumacher I see the PR is still in draft. Is there still work you intend to do here? Typically we wait with reviews until a PR is marked "ready for review". |
muhlemmer
added
the
waiting
| var ErrStartServerListenAndServe = errors.New("server.ListenAndServe():") | ||
| var ErrStartServerShutdown = errors.New("server.Shutdown():") | ||
|
|
||
| func StartServer2(ctx context.Context, port string, opts ...func(*startServerOpts)) { |
There was a problem hiding this comment.
Could you rename this to a more meaningful name? StartServerWithOptions maybe?
Also, it would be useful to have some docs that explain how is StartServer2 different from StartServer
| if options.errorChannel != nil { | ||
| *options.errorChannel <- fmt.Errorf("%w %v", ErrStartServerListenAndServe, err) | ||
| } |
There was a problem hiding this comment.
I'm wondering what's the behaviour in case I call Startserver2() without any option. In that case, errorChannel will be nil. If so, I think the server will shutdown silently without any message to the user of that happening. I would, at least, put a log.Error when errorChannel == nil. Same goes for lines 148 - 150.
|
Hey @jrschumacher can you pls have a look at Marco's feedback/comments? Thank you! |
Close #710
Definition of Ready