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

Generating new arrays with $append/$prepend #127

Closed
TobiasNx opened this issue Feb 3, 2022 · 7 comments · Fixed by #368
Closed

Generating new arrays with $append/$prepend #127

TobiasNx opened this issue Feb 3, 2022 · 7 comments · Fixed by #368

Comments

@TobiasNx
Copy link
Collaborator

TobiasNx commented Feb 3, 2022

We should reintroduce $append and $prepend as creation methods if the array does not exist in a record to add to or create an array.

Was part of: #111
Related to: #93 and #92

@fsteeg
Copy link
Member

fsteeg commented Feb 3, 2022

Will revisit after #102.

Functional review: @TobiasNx
Code review: @blackwinter / @fsteeg

@TobiasNx TobiasNx changed the title Generating new arrays with $append/$prepend Generating new arrays with $append/$prepend Feb 8, 2022
@TobiasNx TobiasNx added the needsIntegrationTest Integration test(s) should be provided. label Feb 28, 2022
TobiasNx added a commit that referenced this issue Feb 28, 2022
@TobiasNx TobiasNx removed the needsIntegrationTest Integration test(s) should be provided. label Mar 1, 2022
@TobiasNx
Copy link
Collaborator Author

TobiasNx commented Jun 13, 2022

Since this is standard behaviour of Catmandu fix I would like when this gets prioritized over other fix tickets.

@TobiasNx
Copy link
Collaborator Author

It seems that this would "only" need adjustment in two code segments, but I could be wrong:

case $append:
array.add(value);
break;

case $append:
referencedValue = Value.newHash().withPathSet(p); // TODO: append non-hash?
array.add(referencedValue);
break;

@fsteeg
Copy link
Member

fsteeg commented Sep 25, 2024

Ready for functional review in branch 127-arrays.

@fsteeg
Copy link
Member

fsteeg commented Sep 27, 2024

Changes in PR should be done, could you test again with current branch 127-arrays?

@TobiasNx
Copy link
Collaborator Author

TobiasNx commented Oct 2, 2024

I tried lobid-resources and all tests did run trough without changes +1

@TobiasNx TobiasNx assigned fsteeg and unassigned TobiasNx Oct 2, 2024
TobiasNx added a commit that referenced this issue Oct 14, 2024
TobiasNx added a commit that referenced this issue Oct 14, 2024
If we want to support the behaviour for creating an array with index number we should revisit this test and change it.
TobiasNx added a commit that referenced this issue Oct 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

3 participants