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

Introduce logging categories #7004

Merged
merged 1 commit into from Oct 11, 2023
Merged

Introduce logging categories #7004

merged 1 commit into from Oct 11, 2023

Conversation

jedelbo
Copy link
Contributor

@jedelbo jedelbo commented Sep 28, 2023

What, How & Why?

☑️ ToDos

  • 📝 Changelog update
  • 🚦 Tests (or not relevant)
  • C-API, if public C++ API changed.

@cla-bot cla-bot bot added the cla: yes label Sep 28, 2023
@jedelbo jedelbo force-pushed the je/core-logging branch 2 times, most recently from 390c801 to 2fde0b2 Compare September 29, 2023 11:13
@coveralls-official
Copy link

coveralls-official bot commented Sep 29, 2023

Pull Request Test Coverage Report for Build github_pull_request_277110

  • 258 of 282 (91.49%) changed or added relevant lines in 13 files are covered.
  • 4847 unchanged lines in 112 files lost coverage.
  • Overall coverage decreased (-2.0%) to 90.896%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/realm/util/logger.cpp 39 40 97.5%
src/realm/replication.cpp 26 28 92.86%
src/realm/transaction.cpp 7 9 77.78%
test/util/compare_groups.cpp 2 4 50.0%
src/realm/util/timestamp_logger.cpp 0 3 0.0%
src/realm/query.cpp 12 19 63.16%
src/realm/util/logger.hpp 74 81 91.36%
Files with Coverage Reduction New Missed Lines %
src/realm/object-store/impl/collection_change_builder.cpp 1 99.41%
src/realm/object-store/impl/list_notifier.cpp 1 98.67%
src/realm/object-store/thread_safe_reference.cpp 1 97.06%
src/realm/group_writer.cpp 2 95.91%
src/realm/object-store/dictionary.hpp 2 95.65%
src/realm/sync/instruction_replication.hpp 2 79.31%
src/realm/sync/network/http.hpp 2 80.87%
src/realm/sync/network/websocket.cpp 2 75.26%
src/realm/sync/noinst/server/server_history.cpp 2 67.69%
src/realm/util/timestamp_logger.cpp 2 0.0%
Totals Coverage Status
Change from base Build github_pull_request_275914: -2.0%
Covered Lines: 234844
Relevant Lines: 258366

💛 - Coveralls

@jedelbo jedelbo marked this pull request as ready for review September 29, 2023 13:16
static LogCategory /**/ transaction; // Creating, advancing and committing transactions
static LogCategory /**/ query; // Query operations
static LogCategory /**/ object; // Mutations of the database
static LogCategory /**/ notification; // Reporting changes to the database
Copy link
Contributor

Choose a reason for hiding this comment

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

storage.notification and all loggers below are not used anywhere. Is something missing?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It will come in a subsequent PR

Copy link
Contributor

Choose a reason for hiding this comment

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

ok, worth a fixme or todo then

{

// FIXME use category
Copy link
Contributor

Choose a reason for hiding this comment

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

so how the capi would change to accommodate this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The logger function must be able to receive a string indicating the category

src/realm/util/logger.cpp Show resolved Hide resolved
@jedelbo jedelbo merged commit 395ebae into next-major Oct 11, 2023
29 of 31 checks passed
@jedelbo jedelbo deleted the je/core-logging branch October 11, 2023 13:03
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 21, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants