sul-dlss / cocina-models

Cocina repository data model (implemented in Ruby)
https://sul-dlss.github.io/cocina-models/
3 stars 0 forks source link

add BCP 47 language tag attribute for files #640

Closed jmartin-sul closed 11 months ago

jmartin-sul commented 11 months ago

NOTE: Changes to openapi.yml require updating openapi.yml for sdr-api and dor-services-app and generating models - see README.

Why was this change made? ๐Ÿค”

closes #637

How was this change tested? ๐Ÿคจ

unit tests

โšก โš  If this change has cross service impact, run integration tests and/or test in [stage|qa] environment, in addition to specs. โšก

jmartin-sul commented 11 months ago

merging since @mjgiarlo approved and i took his non-blocking suggestion.

not urgent, but still interested in feedback for possible follow on touchups re:

mjgiarlo commented 11 months ago

@jmartin-sul ๐Ÿ’ฌ

merging since @mjgiarlo approved and i took his non-blocking suggestion.

๐Ÿ™๐Ÿป

  • docs/description_types.md after running exe/generator generate

I think we ought to take whatever the generator generates (now and always---and if the generator isn't generating the right stuff, we should fix the generator logic).

  • whether attribute? makes optional redundant

I believe attribute? makes the key optional (can be missing) and optional makes a value nilable?

  • whether to up the limit on (or disable) RSpec/ExampleLength

I'm ๐Ÿ’ฏ on-board with the change you made.

jmartin-sul commented 11 months ago

@jmartin-sul ๐Ÿ’ฌ

ok, took that output and added it in the first commit in https://github.com/sul-dlss/cocina-models/pull/641

๐Ÿ™๐Ÿป

at any rate, the tests already ensure that the value can be omitted entirely, so that does seem to indicate that .optional is superfluous in some way in this case.

Thanks for testing this!