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

Document the checklist of all the tasks necessary when making an existing single field multiple #9634

Closed
landreev opened this issue Jun 5, 2023 · 3 comments · Fixed by #10284
Labels
Feature: Developer Guide Size: 3 A percentage of a sprint. 2.1 hours.
Milestone

Comments

@landreev
Copy link
Contributor

landreev commented Jun 5, 2023

The recently merged PRs that made the formerly single value-only fields "productionPlace" and "series" multiple (#9254 and #9256 respectively), introduced A LOT of problems in the export and import code that the core team had to spend a lot of time addressing after the fact. Some were not even caught in time (for example, the "all fields" sample json file that's linked in the v5.13 (latest release as of now) guide (dataset-create-new-all-default-fields.json) has an entry for productionPlace that is encoded as single-only, thus making the json un-importable once the citation block is updated as part of a v5.13 install or upgrade. The most recent example: #9633.

It would not be fair to blame the developer for the oversights. It's not obvious or intuitive that all these changes need to be made and where. And we apparently don't have enough automated tests to alert a developer of all these problems. So we need to document the checklist of all the required tasks in the developers guide. And then if (God forbid) anyone ever needs to make another existing single-only field multiple, it would be easy to make it all 100% responsibility of the developer.

(It says something about our metadata import/export framework, that it requires all these explicit code fixes to accommodate such a change - ideally, all that behavior could be made to be derived from the metadata block configuration (?)... but that is absolutely outside of the scope of this PR).

We could add a couple of extra integration tests though to detect some of the problems in question. For example, it would be trivial to add an import test for our provided sample file mentioned above to ensure that it stays importable. That can be handled as part of this issue as well.

@cmbz
Copy link

cmbz commented Jun 7, 2023

Although I'm not sure this is ideal, I added the Help Wanted: Documentation tag and added it to the Global Backlog. This issue feels more urgent to me than the tag indicates, but I'm not certain who is best positioned to make the change; I think that it should be prioritized.

@cmbz
Copy link

cmbz commented Aug 17, 2023

2023/08/17: Moved to Dataverse Team queue for prioritization.

@cmbz cmbz added the Size: 3 A percentage of a sprint. 2.1 hours. label Dec 18, 2023
@cmbz
Copy link

cmbz commented Dec 18, 2023

2023/12/18:

  • Sized at a 3 during sizing meeting.
  • Note: the recent PR that made the Alternative Title multiple should be used as a model for documenting this checklist in the guide:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature: Developer Guide Size: 3 A percentage of a sprint. 2.1 hours.
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

3 participants