Skip to content

Switch to slog - #317

Open
lucacome wants to merge 1 commit into
ncabatoff:masterfrom
lucacome:slog
Open

lucacome wants to merge 1 commit into
ncabatoff:masterfrom
lucacome:slog

Conversation

@lucacome

Copy link
Copy Markdown

This pull request includes several updates and improvements across multiple files, focusing on upgrading dependencies, enhancing logging, and refactoring code for better readability and maintainability. Below are the most important changes grouped by theme:

Dependency Upgrades:

  • Updated the actions/setup-go action to version 5 and set the Go version to stable in .github/workflows/build.yml and .github/workflows/release.yml. [1] [2]
  • Updated the goreleaser/goreleaser-action to version 6 in .github/workflows/release.yml.

Configuration and Build:

  • Added a new configuration file .golangci.yml to enable specific linters and set exclusion rules for test files.
  • Updated the Makefile to use the -trimpath flag in the build target and upgraded the Go version in dockertest and dockerinteg targets. [1] [2]

Logging Enhancements:

  • Replaced the standard log package with log/slog for structured logging in cmd/process-exporter/main.go and collector/process_collector.go. [1] [2]
  • Updated various logging statements to use the new logger and improved error handling in cmd/process-exporter/main.go. [1] [2]

Code Refactoring:

  • Refactored flag handling in cmd/process-exporter/main.go to use kingpin for better command-line argument parsing.
  • Replaced deprecated functions ioutil.TempFile and ioutil.ReadFile with os.CreateTemp and os.ReadFile respectively in cmd/load-generator/main.go and config/config.go. [1] [2]

Miscellaneous:

  • Added build constraint //go:build linux to cmd/load-generator/main.go for specifying the target operating system.
  • Simplified variable declarations and error handling in cmd/integration-tester/main.go. [1] [2]

These changes collectively aim to modernize the codebase, improve logging, and ensure compatibility with the latest tools and dependencies.

@ncabatoff

Copy link
Copy Markdown
Owner

Thanks @lucacome, this is a generous contribution. I'm sorry I didn't get around to looking at it until now. Part of the issue is that it's a bunch of unrelated changes. Would you mind splitting it into at least two PRs - one for the dependency updates/configuration/build stuff, and one for the Go changes? Ideally I'd like to see the kingpin change in its own PR too, as that's the one I'm least certain I want.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants