Skip to content

[#21,#24] Replace rodsLog, inline code where appropriate - #36

Merged
korydraughn merged 2 commits into
irods:masterfrom
MartinFlores751:21.m
Mar 11, 2026
Merged

[#21,#24] Replace rodsLog, inline code where appropriate#36
korydraughn merged 2 commits into
irods:masterfrom
MartinFlores751:21.m

Conversation

@MartinFlores751

@MartinFlores751 MartinFlores751 commented Mar 4, 2026

Copy link
Copy Markdown
Contributor

Issue quick links: #21, #24

This PR replaces rodsLog with the newer logger under policy_composition_category. This PR also adds inline where appropriate to the policy comp framework library.

@korydraughn

korydraughn commented Mar 4, 2026

Copy link
Copy Markdown
Contributor

Will review once this is out of draft.

MartinFlores751
MartinFlores751 commented Mar 4, 2026
View reviewed changes

@MartinFlores751 MartinFlores751 left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

PR should be good to review, will pull out of draft. My comment is on the part that I need to update still. Will need eyes on the inline code. I'm not sure how generous I should be with the keyword.

Comment thread framework/src/policy_composition_framework_utilities.cpp Outdated
@MartinFlores751
MartinFlores751 marked this pull request as ready for review March 4, 2026 20:13
alanking
alanking reviewed Mar 4, 2026
View reviewed changes
Comment thread framework/example/libirods_rule_engine_plugin-policy_engine_example.cpp Outdated
Comment thread framework/src/policy_composition_framework_utilities.cpp Outdated
Comment thread libirods_rule_engine_plugin-policy_engine-filesystem_usage.cpp Outdated
Comment thread parameter_substitution.hpp Outdated
Comment thread parameter_substitution.hpp Outdated
Comment thread framework/include/irods/policy_composition_framework_logging_category.hpp Outdated
MartinFlores751
MartinFlores751 commented Mar 4, 2026
View reviewed changes

@MartinFlores751 MartinFlores751 left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Live review with @korydraughn.

Comment thread parameter_substitution.hpp Outdated
Comment thread libirods_rule_engine_plugin-policy_engine-query_processor.cpp Outdated
Comment thread framework/include/irods/policy_composition_framework_policy_engine.hpp Outdated
Comment thread libirods_rule_engine_plugin-policy_engine-filesystem_usage.cpp Outdated
Comment thread framework/include/irods/policy_composition_framework_logging_category.hpp Outdated
korydraughn
korydraughn reviewed Mar 5, 2026
View reviewed changes
Comment thread libirods_rule_engine_plugin-policy_engine-testing_policy.cpp Outdated
Comment thread libirods_rule_engine_plugin-event_handler-data_object_modified.cpp Outdated
alanking
alanking reviewed Mar 5, 2026
View reviewed changes
Comment thread framework/include/irods/policy_composition_framework_policy_engine.hpp Outdated
korydraughn
korydraughn reviewed Mar 5, 2026
View reviewed changes
Comment thread libirods_rule_engine_plugin-policy_engine-log_context.cpp Outdated
@korydraughn

korydraughn commented Mar 5, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Tests are passing so far. Things are looking good.

I assume we're waiting for tests to pass and then we'll be ready to squash. You agree?

@MartinFlores751

MartinFlores751 commented Mar 5, 2026

Copy link
Copy Markdown
Contributor Author

Tests are passing so far. Things are looking good.

I assume we're waiting for tests to pass and then we'll be ready to squash. You agree?

Yep. Everything's been addressed, so it's just waiting for the tests.

@korydraughn

korydraughn commented Mar 5, 2026

Copy link
Copy Markdown
Contributor

After we get this PR in, all that's left to do is ...

  • Consider renaming the repository
  • Add a changelog
  • Update the README with anything that might ease the learning curve

@korydraughn

korydraughn commented Mar 5, 2026

Copy link
Copy Markdown
Contributor

All of the tests passed.

Take a look at the clang-tidy report if you haven't already. See if we can hammer those out.

@MartinFlores751 MartinFlores751 mentioned this pull request Mar 9, 2026
Closed
korydraughn
korydraughn reviewed Mar 11, 2026
View reviewed changes

@korydraughn korydraughn left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good to me.

After you make the change, squash to taste.

Comment thread framework/include/irods/policy_composition_framework_plugin_configuration_json.hpp Outdated
@MartinFlores751

MartinFlores751 commented Mar 11, 2026

Copy link
Copy Markdown
Contributor Author

Squashed.

@MartinFlores751

MartinFlores751 commented Mar 11, 2026

Copy link
Copy Markdown
Contributor Author

Did a quick edit to commit message.

korydraughn
korydraughn approved these changes Mar 11, 2026
View reviewed changes

@korydraughn korydraughn left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pound it.

@MartinFlores751
[irods#24] Inline everything defined in .hpp in framework lib
9c3aca4
@MartinFlores751
[irods#21] Add new logging category
a86ff22 
This commit adds a common logging category for policy composition
plugins. The plugins were updated to use this new logger as well.

This commit also contains several fixes:
- Fixing `.dump(...)` calls to handle invalid Unicode sequences
- Adding additional details, such as func `__func__`, to logs
- Fixing includes, macro redefinition, and other tidy issues
@MartinFlores751

MartinFlores751 commented Mar 11, 2026

Copy link
Copy Markdown
Contributor Author

Pounded.

@korydraughn
korydraughn merged commit 9533463 into irods:master Mar 11, 2026
18 of 19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@korydraughn korydraughn korydraughn approved these changes
+1 more reviewer
@alanking alanking alanking left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@MartinFlores751 @korydraughn @alanking