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

participationFees: Adding id to changed merging behavior #118

Closed
jpmckinney opened this issue Nov 12, 2019 · 6 comments
Closed

participationFees: Adding id to changed merging behavior #118

jpmckinney opened this issue Nov 12, 2019 · 6 comments
Labels
bug Core Relates to a recommended extension Schema Involves editing the schema
Milestone

Comments

@jpmckinney
Copy link
Member

jpmckinney commented Nov 12, 2019

Before 1.1.3, participation fees had no id field, which meant the array was merged as a whole. An id field was added in 1.1.3 #48, per the schema style guide at the time.

However, this had the effect of changing the merging behavior to the identifier merge strategy.

To restore the original behavior, we can either:

  1. Remove the id field
  2. Add "wholeListMerge": true

@duncandewhurst @yolile What is your preference?

@jpmckinney jpmckinney added the Core Relates to a recommended extension label Nov 12, 2019
@jpmckinney jpmckinney added this to the 1.1.5 milestone Nov 12, 2019
@jpmckinney jpmckinney added bug Schema Involves editing the schema labels Nov 12, 2019
@jpmckinney
Copy link
Member Author

jpmckinney commented Nov 12, 2019

I double checked, and I don't think any other id fields were added to extensions at that time (grepping for \*.*add.*\bid\b).

@yolile
Copy link
Member

yolile commented Nov 13, 2019

@jpmckinney right now, without the id field, if you have more than one item in the participation fees array CoVe report them as duplication (even if they are different)

@jpmckinney
Copy link
Member Author

Can you report that issue on the CoVE repo?

@yolile
Copy link
Member

yolile commented Nov 13, 2019

Already reported OpenDataServices/cove#1246 for the array of strings case, but now updated with this case too

@jpmckinney jpmckinney changed the title Adding id to participation fees changed merging behavior Participation fees: Adding id to changed merging behavior Nov 13, 2019
@jpmckinney jpmckinney changed the title Participation fees: Adding id to changed merging behavior participationFees: Adding id to changed merging behavior Nov 13, 2019
@duncandewhurst
Copy link

I prefer 2. to keep consistency of structure (if not merging strategy) with other arrays.

Also, if anyone extended participation fees to include a property which is an array then an id field would be necessary for flattening.

@jpmckinney
Copy link
Member Author

Oops, we actually already set wholeListMerge in 1.1.4. No issue here, then!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Core Relates to a recommended extension Schema Involves editing the schema
Projects
Archived in project
Development

No branches or pull requests

3 participants