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

feat(appconfig): Only log lazy-appconfig when an app can do something… #48821

Merged
merged 1 commit into from
Nov 6, 2024

Conversation

nickvergessen
Copy link
Member

… about it

💡 Ideas

  • Only log when a specific app triggered the lazy-load
  • Allow to limit logging to specific app:
    • Add a general "debug app" setting
      • 👍 Could be reused in other places, e.g. logging of throwing old activity/notification exceptions, etc.
      • 👎 Could be very generic at some point and does not allow an app dev to check specific for 1 case (could still be done by log filtering)
    • Specific setting log_lazy_appconfig_apps to only log lazy-appconfig loading from a list of apps
      • 👍 Can be easily targeted
      • 👎 If we do similar things for other features we build up a huge list of dev related system configs
    • Reuse "log_condition" logic
      • 👍 Already existing concept
      • 👎 Might make all devs use log-condition and everyone becomes blind for logs from other apps where they could shoot a drive-by PR

Checklist

@nickvergessen nickvergessen added this to the Nextcloud 31 milestone Oct 20, 2024
@nickvergessen nickvergessen self-assigned this Oct 20, 2024
@nickvergessen nickvergessen requested review from a team, icewind1991 and nfebe and removed request for a team November 2, 2024 21:46
@nickvergessen nickvergessen force-pushed the techdebt/noid/more-useful-debug-logs branch 2 times, most recently from ff70ee9 to ceb41e2 Compare November 6, 2024 13:03
lib/private/AppConfig.php Outdated Show resolved Hide resolved
… about it

Signed-off-by: Joas Schilling <coding@schilljs.com>
@nickvergessen nickvergessen force-pushed the techdebt/noid/more-useful-debug-logs branch from ceb41e2 to f237846 Compare November 6, 2024 13:16
@ChristophWurst ChristophWurst added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Nov 6, 2024
@nickvergessen nickvergessen merged commit 4bf6cd8 into master Nov 6, 2024
177 checks passed
@nickvergessen nickvergessen deleted the techdebt/noid/more-useful-debug-logs branch November 6, 2024 15:30
@skjnldsv skjnldsv mentioned this pull request Jan 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4. to release Ready to be released and/or waiting for tests to finish technical debt
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants