Skip to content

Conversation

@sdn4z
Copy link
Collaborator

@sdn4z sdn4z commented Nov 6, 2025

We can now manage verbosity via a -v flag in the cli.
There's been also a minor refactor, that deletes de Settings class while keeping the same functionality, and some other minor changes.

@sdn4z sdn4z force-pushed the logging branch 3 times, most recently from 1c28085 to 84a04f9 Compare November 6, 2025 11:27
@sdn4z
Copy link
Collaborator Author

sdn4z commented Nov 6, 2025

/lgtm review

@github-actions github-actions bot added feature and removed feature labels Nov 6, 2025
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🦉 lgtm Review

Score: Needs a Lot of Work 🚨

🔍 Summary

This PR makes some nice improvements by adding a -v flag for verbose output and refactoring the configuration. The centralization of constants and added logging are also welcome changes.

However, there are a few significant issues that need to be resolved before this can be merged. The main concerns are regressions in error handling and test coverage:

  1. Error Handling: The application can now crash if Sentry fails to initialize or if an invalid LOG_LEVEL is set in the environment. We should restore the previous robust error handling.
  2. Test Coverage: Key assertions that verified logging output have been removed from the tests. For a PR that changes logging, it's essential to maintain or increase this test coverage to ensure everything works as expected.

Please address the comments below to fix these regressions. Once these are resolved, the PR will be in good shape.

More information
  • Id: d3094796d8f5434eab07222a76fdff31
  • Model: gemini-2.5-pro
  • Created at: 2025-11-06T11:30:38.184457+00:00
Usage summary
  • Request count: 2
  • Request tokens: 105,719
  • Response tokens: 16,603
  • Total tokens: 122,322

See the 📚 lgtm-ai repository for more information about lgtm.

@sdn4z sdn4z marked this pull request as ready for review November 6, 2025 11:54
@sdn4z sdn4z merged commit 181d7d0 into elementsinteractive:main Nov 6, 2025
10 checks passed
@sdn4z sdn4z deleted the logging branch November 6, 2025 11:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant