Actually using the new product_query (Part 1)#344
Actually using the new product_query (Part 1)#344beojan wants to merge 33 commits intoFramework-R-D:mainfrom
Conversation
Won't build yet
This handles the API change, and ports the existing creator name check to work with the new product_query.
Also use std::unreachable() instead of kind = "HOW??"s;
- Introduce `identifier` - Combine `product_query` and `product_tag` - Express above in terms of `identifier` - Changes to make everything work again
Mostly compilation errors, plus exception change in configuration test
Codecov Report❌ Patch coverage is @@ Coverage Diff @@
## main #344 +/- ##
==========================================
+ Coverage 85.36% 86.64% +1.28%
==========================================
Files 122 122
Lines 2433 2494 +61
Branches 389 394 +5
==========================================
+ Hits 2077 2161 +84
+ Misses 233 211 -22
+ Partials 123 122 -1
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 6 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
beojan
changed the title
|
@phlexbot format |
|
No automatic header-guards fixes were necessary. |
|
No automatic markdownlint fixes were necessary. |
|
No automatic jsonnetfmt fixes were necessary. |
|
No automatic cmake-format fixes were necessary. |
|
Automatic clang-format fixes pushed (commit 3c48ce0). |
| // TODO: Getting there | ||
| if (producer.node.plugin() == identifier(query.creator) || | ||
| producer.node.algorithm() == identifier(query.creator)) { |
There was a problem hiding this comment.
Do we need the explicit casts to identifier? (Also suggest removing the TODO...or be more explicit about what's missing)
|
|
||
| algorithm_name product_store::default_source() | ||
| { | ||
| using namespace literals; |
There was a problem hiding this comment.
Can remove this using directive.
| #include <vector> | ||
|
|
||
| using namespace phlex::experimental; | ||
| using namespace phlex::experimental::literals; |
There was a problem hiding this comment.
This using directive is not needed.
|
|
||
| void cells_to_process(experimental::async_driver<data_cell_index_ptr>& d) | ||
| { | ||
| using namespace experimental::literals; |
There was a problem hiding this comment.
This using directive is not necessary.
| private: | ||
| std::string content_; | ||
| std::uint64_t hash_; | ||
| std::uint64_t hash_{0}; |
There was a problem hiding this comment.
Is 0 the calculated hash of the empty string?
There was a problem hiding this comment.
No, it isn't. This is only here so I can = default the default constructor without risking undefined behaviour. And the default constructor is only there so other classes that contain identifiers can be default constructed.
| // TODO: This needs to be replaced with a properly engineered solution | ||
| // return msg.store->get_handle<handle_arg_t>(query.spec()); |
There was a problem hiding this comment.
Why does get_handle<...>(query.spec()) not work?
2 participants
Mostly switching to using
identifiermembers, along with adding the ability to resolve products without a suffix.