Created
February 22, 2025 10:17
-
-
Save denarced/702b79e5dfcc276e760674532b3df896 to your computer and use it in GitHub Desktop.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| diff --git a/cmd/golinks/golinks.go b/cmd/golinks/golinks.go | |
| index 076c4b7..d5402d5 100644 | |
| --- a/cmd/golinks/golinks.go | |
| +++ b/cmd/golinks/golinks.go | |
| @@ -18,6 +18,7 @@ import ( | |
| func main() { | |
| cfg := config.GetConfig() | |
| + // Isn't this redundant since "router.New()" does this very same initialization? | |
| logger := log.Output(zerolog.ConsoleWriter{ | |
| Out: os.Stdout, | |
| TimeFormat: time.RFC3339Nano, | |
| diff --git a/config/config.go b/config/config.go | |
| index a548616..e6bae52 100644 | |
| --- a/config/config.go | |
| +++ b/config/config.go | |
| @@ -56,6 +56,7 @@ respected, though full paths are.` | |
| } | |
| func getLevelFromArg(arg string) zerolog.Level { | |
| + // You're intentionally not using zerolog.ParseLevel(arg)? | |
| switch strings.ToUpper(arg) { | |
| case "TRACE": | |
| return zerolog.TraceLevel | |
| diff --git a/internal/handler/feHandler.go b/internal/handler/feHandler.go | |
| index 20a1fb9..c6aad4e 100644 | |
| --- a/internal/handler/feHandler.go | |
| +++ b/internal/handler/feHandler.go | |
| @@ -8,6 +8,8 @@ import ( | |
| "net/http" | |
| ) | |
| +// Nice! I didn't know about this. | |
| + | |
| //go:embed static/* | |
| var content embed.FS | |
| @@ -27,6 +29,7 @@ var staticContent = map[ContentName]string{ | |
| FAVICON: "static/favicon.ico", | |
| } | |
| +// Does "FE" stand for "file embed"? | |
| type FeHandler struct{} | |
| func NewFeHandler() *FeHandler { | |
| diff --git a/internal/handler/goLinkService.go b/internal/handler/goLinkService.go | |
| index 3d672dd..a03b9d0 100644 | |
| --- a/internal/handler/goLinkService.go | |
| +++ b/internal/handler/goLinkService.go | |
| @@ -15,6 +15,7 @@ type GolinkHandler struct { | |
| feHandler *FeHandler | |
| } | |
| +// Doesn't return anything. | |
| // NewGoLinkService returns a reference to a new instance of a GolinkHandler | |
| func NewGoLinkService(router *chi.Mux, cfg *config.Config) { | |
| linkMap := links.NewLinkMap(cfg.StorageType, cfg.ConfigFile) | |
| diff --git a/internal/links/links.go b/internal/links/links.go | |
| index 83dd1f2..5c74119 100644 | |
| --- a/internal/links/links.go | |
| +++ b/internal/links/links.go | |
| @@ -52,6 +52,7 @@ func (l *LinkMap) handleReload() { | |
| return | |
| } | |
| + // Why select? Why not just for-range? | |
| for { | |
| select { | |
| case signal := <-reloadChannel: | |
| diff --git a/internal/links/storage/file.go b/internal/links/storage/file.go | |
| index 67c5507..ddc70cb 100644 | |
| --- a/internal/links/storage/file.go | |
| +++ b/internal/links/storage/file.go | |
| @@ -93,6 +93,9 @@ func findConfig(requestedConfig string) string { | |
| } | |
| file, err := openFile(config) | |
| if err == nil { // Note the deviation from the standard err != nil | |
| + // Generally string concatenation is discouraged because it's always done, even when | |
| + // nothing is logged. Here it doesn't matter. A few dozen nanoseconds here or there (I | |
| + // didn't actually measure but I remember figures like that with Log4J). | |
| log.Info().Msg("Using config file " + config) | |
| file.Close() | |
| return config | |
| @@ -104,6 +107,7 @@ func findConfig(requestedConfig string) string { | |
| log.Err(err) | |
| } | |
| log.Fatal().Msg("Failed to find any config") | |
| + // Isn't this irrelevant sice "log.Fatal()" already calls exit? | |
| panic(errs) | |
| } |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment