Skip to content
This repository has been archived by the owner on Jan 27, 2025. It is now read-only.

Current state of Metafix implementation #60

Merged
merged 59 commits into from
Nov 9, 2021
Merged

Current state of Metafix implementation #60

merged 59 commits into from
Nov 9, 2021

Conversation

fsteeg
Copy link
Member

@fsteeg fsteeg commented Oct 14, 2021

As discussed in our recent meeting, we want to merge the current state of the Metafix implementation into the main branch to make clear where the main development is happening.

Would it make sense to resolve #35 with this and open new issues for specific improvements / new features?

@blackwinter Feel free to merge yourself as I'll be on vacation starting tomorrow (back on the 25th).

fsteeg added 30 commits July 2, 2021 15:39
@fsteeg
Re-approach record-based Metafix implementation (#35)
f328a13 
Starting from a record map, not the event-based morph classes

Following Catmandu Fix cheat sheet at
https://github.com/LibreCat/Catmandu/wiki/Fixes-Cheat-Sheet
@fsteeg
Implement elsif and else blocks (#35)
3045dbb 
See https://github.com/LibreCat/Catmandu/wiki/Conditionals
@fsteeg
Implement unless conditional (#35)
6981259 
See https://github.com/LibreCat/Catmandu/wiki/Conditionals
@fsteeg
Basic list bind implementation (#35)
e7b872a 
See https://github.com/LibreCat/Catmandu/wiki/Binds
@fsteeg
Update flux-commands.properties (#35)
ebcf650 
MetafixFilter was removed in f328a13
@fsteeg
Rename non-generated fix package to metafix (#35)
a786f73
@fsteeg
Move metamorph package to metafix (#35)
191351b
@fsteeg
Add array- and hash-related record level methods (#35)
7ba8f99 
And TODOs for internal record representation as JSON-equivalent
@fsteeg
Implement format, parse_text, and paste (#35)
4659439 
See https://github.com/LibreCat/Catmandu/wiki/Fixes-Cheat-Sheet
@fsteeg
Switch internal record map to Object values (#35)
0d903b3
@fsteeg
Emit Map values in record as entities (#35)
f59faf5
@fsteeg
Basic implementation of exists and reject (#35)
92fdf53 
See https://github.com/LibreCat/Catmandu/wiki/Fixes-Cheat-Sheet
@fsteeg
Emit arrays as entities, support .$append (#35)
506e5fd 
See https://github.com/LibreCat/Catmandu/wiki/Fixes-Cheat-Sheet
@fsteeg
Implement retain method (#35)
b022b51
@fsteeg
Skip non-existing fields in paste (#35)
78c9ec9
@fsteeg
Implement vacuum method (#35)
998da3d
@fsteeg
Add disabled tests for list binds and entity representation (#35)
b5604b8 
Flattened field names vs. actual entities, both for emitting and
internal representation (apply actual JsonPath to the record?)
@fsteeg
Switch internal record from Multimap to Map<String,Object> (#35)
db1c437
@fsteeg
Support dot notation for nested entities in set_field (#35)
cea496e
@fsteeg
Support dot notation for nested entities in add_field (#35)
46719da
@fsteeg
Rename class for testing record level methods (#35)
8e104a8
@fsteeg
Don't flatten incoming entites, use nested maps internally (#35)
c5343a8 
WIP: 4 failing tests with TODO comments for missing entity support
@fsteeg
Support dot notation for nested maps in remove_field (#35)
f08252a 
WIP: 3 failing tests with TODO comments for missing entity support
@fsteeg
Support dot notation for nested maps in copy and move (#35)
c36235c 
WIP: 2 failing tests with TODO comments for missing entity support
@fsteeg
Support dot notation for nested maps in predicates (#35)
e19352c 
Used in `if` statements with `(all|any|none)_(contain|equal|match)`
@fsteeg
Support dot notation for nested maps in path option of bind (#35)
e4c440a
@fsteeg
Support dot notation for nested maps in paste method (#35)
e1b06fb
@fsteeg
Merge repeated entities into single map (#35)
878fd8d
@fsteeg
Fix output for arrays, always emit like field[].1 (#35)
e5b395a
@fsteeg
Fix array processing issues, work on $append & $last (#35)
b6af4a1
@blackwinter
Fix named group access in parse_text. (5395e90)
26d2a9e 
Access group by name instead of index (which may be off when used in combination with unnamed groups).
@blackwinter
Fix Checkstyle violations in MetafixMethodTest. (26d2a9e)
bdc43ee
@blackwinter blackwinter mentioned this pull request Nov 5, 2021
Closed
@fsteeg
Restore FixServlet and ServerLauncher (#35)
e4dec3a 
Accidentally deleted during refactoring in a786f73
@fsteeg
Copy link
Member Author

fsteeg commented Nov 5, 2021

Why were ServerLauncher and FixServlet removed in a786f73?

Hm, that must have happened accidentally during refactoring, I restored them in e4dec3a.

Why was MetafixFilter removed in f328a13? [...] commit without any mention of the removal

Right, that should have been a separate commit, but was intentionally. My idea was that we don't need a separate filter command with fix, since we can filter in the main fix (or in a separate fix, but with normal fix behavior), e.g. like this.

This was referenced Nov 5, 2021
Closed
Closed
@fsteeg
Copy link
Member Author

fsteeg commented Nov 5, 2021

I would suggest to create a Record class that wraps the Map<String, Object> instead of "littering" Metafix and FixMethod with static methods.

Yes, good point. I opened #64 for that.

@blackwinter blackwinter mentioned this pull request Nov 5, 2021
Closed
@blackwinter
Copy link
Member

My idea was that we don't need a separate filter command with fix, since we can filter in the main fix (or in a separate fix, but with normal fix behavior)

Agreed, reject() is a viable alternative.

@fsteeg fsteeg mentioned this pull request Nov 5, 2021
Closed
@fsteeg
Copy link
Member Author

fsteeg commented Nov 5, 2021

@blackwinter Alright, I hope I have addressed or opened issues for all your points. Reassigning & re-requesting review.

@fsteeg fsteeg requested a review from blackwinter November 5, 2021 14:32
@fsteeg fsteeg assigned blackwinter and unassigned fsteeg Nov 5, 2021
@blackwinter
Copy link
Member

I hope I have addressed or opened issues for all your points.

Yes, I believe everything's been addressed. Thank you! 😄

Once the conversation regarding the binding scope in do list has been resolved (either by opening an issue or, preferably, by changing the implementation), this pull request can be merged. I'll hold off on the final approval until then.

@fsteeg fsteeg mentioned this pull request Nov 8, 2021
Closed
blackwinter
blackwinter approved these changes Nov 8, 2021
View reviewed changes
Copy link
Member

@blackwinter blackwinter left a comment

Choose a reason for hiding this comment

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

OK, let's move on then :)

@blackwinter blackwinter assigned fsteeg and unassigned blackwinter Nov 8, 2021
@blackwinter blackwinter mentioned this pull request Nov 8, 2021
Closed
@fsteeg
Rename Gradle project from metafacture-fix to metafix (#35)
1fe336b 
For consistency: org.metafacture:metamorph org.metafacture:metafix
@fsteeg fsteeg merged commit 2cac372 into master Nov 9, 2021
@fsteeg fsteeg deleted the 35-metafix branch November 9, 2021 09:38
@blackwinter blackwinter mentioned this pull request Nov 9, 2021
Closed
fsteeg added a commit to metafacture/metafacture-playground that referenced this pull request Nov 11, 2021
@fsteeg
Update GitHub Actions and README for using fix from master
92d152b 
See metafacture/metafacture-fix#60
blackwinter added a commit that referenced this pull request Nov 16, 2021
@blackwinter
Remove non-functional code for finding values in nested arrays.
7a8b800 
Seems to be currently unused and doesn't do what it's supposed to do.

See #60 (comment).
blackwinter added a commit that referenced this pull request Nov 16, 2021
@blackwinter
Remove non-functional code for finding values in nested arrays.
ebc00e7 
Seems to be currently unused and doesn't do what it's supposed to do.

See #60 (comment).
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@blackwinter blackwinter blackwinter approved these changes

Assignees

@fsteeg fsteeg

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Provide (non-field-streaming) record mode
2 participants
@fsteeg @blackwinter