Conversation
TheLazzziest
left a comment
There was a problem hiding this comment.
So far it looks great, but there is some room for improvements.
|
|
Refactored test fixtures to use asset templates
Did not use Next steps if this looks good:
|
There was a problem hiding this comment.
It looks much better now. Just don't forget that about glob functionality itself:
- Matching Any String
example.*.models - Matching a Single Character
example.fo?.models - Matching character groups:
- Character classes
example.api.v[12].models - Character ranges
example.api.v[0-9].models
- Character classes
- Complementation
- Character class
example.api.v[!1].models - Character range
example.api.v[!0-9].models
- Character class
- Pathnames:
- Multiple packages deep:
example.v?.*.*.models - Recursive packages lookup:
example.**.api.*.models
- Multiple packages deep:
- Errors:
- Missing rule:
example.v?..models - Invalid delimiter
example.v?,,models
- Missing rule:
As for tables, there is no need so far. It will be too time consuming to implement. We still don't know if the maintainer will share this approach. So my suggestion would be the following:
- Complete the test suit
- Implement the first two cases (Any string, A single char)
- Open a drafted PR with a question (if this approach is ok), then continue work if he says - yes.
|
Thanks for the feedback! I've implemented full support for all the requested glob pattern cases:
The code has been refactored into a dedicated |
Summary: An introduction of a state machineThe current implementation treats module discovery as a graph traversal problem solved with a Breadth-First Search (BFS) using a state machine.
TradeoffsAdvantages
Risks & Limitations
|
|
A small recap of what has been done:
Remaining scope of the work:
|
…to feature/glob-pattern-support
|
Separate graph building from serialization and add dynamic graph comparison This commit refactors the graph generation logic to separate graph building ChangesCore Refactoring
Namespace Packages Support
Test Fixes
Technical DetailsWhy separate serialization?Previously,
This made it impossible to:
Now the flow is:
Why re-merge metadata after imports?When merging metadata from multiple namespace packages:
This ensures the final merged metadata contains all tables from all Testing
All changes are backward compatible. |
|
LGTM! |
No description provided.