search

metafacture / metafacture-fix

Work in progress towards an implementation of the Fix language for Metafacture
Apache License 2.0
6 stars 2 forks source link

Default behaviour of `copy_field`/`move_field`/`add_field` should be destructive, the appending behaviour creates problems #116

Closed TobiasNx closed 1 year ago

TobiasNx commented 2 years ago

See here (The error seems to be not virulent any more, but the underlying behaviour is still productive.)

Catmandu Fix default behaviour is destructive

Appending should only be done explicitly and copy/move/add_field should be destructive. Could be that this is the root of #113

blackwinter commented 2 years ago

Catmandu is destructive: hbz/Catmandu@4d2f0e3.

fsteeg commented 2 years ago

To avoid conflict with #102, should be started after the planned first PR for #102.

Functional review: @TobiasNx Code review: @fsteeg

TobiasNx commented 2 years ago

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

blackwinter commented 1 year ago

@TobiasNx: add_field() behaviour is going to be changed with #308. Please test it.

TobiasNx commented 1 year ago

@TobiasNx: add_field() behaviour is going to be changed with #308. Please test it.

@blackwinter tested it. seems to work :) +1

blackwinter commented 1 year ago

@TobiasNx: copy_field()/move_field() behaviour is going to be changed with #313. Please test it.

TobiasNx commented 1 year ago

+1 tested different variations. Thanks a lot!

blackwinter commented 1 year ago

tested different variations.

Thanks, but just to be sure: Have you also verified that your current transformations (lobid-resources, oersi, etc.) are going to work with these changes? If not, do you maybe want to do it before we merge?

(The required changes in Limetrans were quite extensive: hbz/limetrans@f992bb4 & hbz/limetrans@c7b7dfa)

TobiasNx commented 1 year ago

tested different variations.

Thanks, but just to be sure: Have you also verified that your current transformations (lobid-resources, oersi, etc.) are going to work with these changes? If not, do you maybe want to do it before we merge?

(The required changes in Limetrans were quite extensive: hbz/limetrans@f992bb4 & hbz/limetrans@c7b7dfa)

I did not test this before but now i did. But I always assumed the destructiv behaviour to be valid and wrote my scripts accordingly. lobid resources seems to work well. oersi too.

TobiasNx commented 1 year ago

(I updated my comment, I did test it. :) )

TobiasNx commented 1 year ago

+1