search

MarcusBarnes / mik

The Move to Islandora Kit is an extensible PHP command-line tool for converting source content and metadata into packages suitable for importing into Islandora (or other digital repository and preservations systems).
GNU General Public License v3.0
34 stars 11 forks source link

Inconsistent default configuration between toolchains #461

Closed bondjimbond closed 6 years ago

bondjimbond commented 6 years ago

According to the Wiki, the CSV Newspapers and Books likes to have child object sequences denoted with a hyphen and a number:

├── book0001 │ ├── page-01.tif │ ├── page-02.tif │ ├── page-03.tif │ ├── page-04.tif │ ├── page-05.tif │ ├── page-06.tif │ ├── page-07.tif │ └── page-08.tif

But CSV Compound's default delimiter is underscore, and needs special configuration to change it:

input_directory/ ├── compounddobject1 │ ├── image_01.tif │ └── image_02.tif ├── compounddobject2 │ ├── image_01.tif │ ├── image_02.tif │ ├── image_03.tif │ └── image_04.tif

Can the defaults (i.e. without any configuration) be changed so that they're consistent between toolchains?

mjordan commented 6 years ago

For compound objects, the separator is set at https://github.com/MarcusBarnes/mik/blob/master/src/writers/CsvCompound.php#L51. I have no issue making the defaults consistent, other than changing default behavior can introduce problems. But, as long as we document the change to compound clearly, I'm OK with it. @MarcusBarnes what do you think?

MarcusBarnes commented 6 years ago

@mjordan It's probably better to make the defaults consistent and document the changes. Maybe we should get in the habit of creating point releases more regularly? That way those using MIK can stick to a specific release tag (unless they need any of the changes introduced).

mjordan commented 6 years ago

Sounds good to me. Who wants to open a PR to fix the compound default? I'm happy to.

bondjimbond commented 6 years ago

@mjordan Just reminding you that this issue is still open, since nobody claimed the PR task.

mjordan commented 6 years ago

OK, will take a look this weekend.

mjordan commented 6 years ago

Working on this, getting tangled up in PHPUnit tests.

mjordan commented 6 years ago

I'll open a PR now. I'm also going to mark the testCsvCompoundInputValidator() test as skipped, since there's obviously something larger going on with the CSV compound and book toolchains. Will open a separate issue for that.

mjordan commented 6 years ago

https://github.com/MarcusBarnes/mik/wiki/Toolchain:-CSV-compound-objects updated to note the change in default value.

bondjimbond commented 6 years ago

@mjordan Was it updated? I'm still seeing underscore on the wiki page.

mjordan commented 6 years ago

Yeah... should have been more specific there. By "updated" I added the text

Note: As of commit 420ccbb22c461e3aef7ac4fec67c0982681f2de1 (April 24, 2018), the default for this value changed from an underscore (_) to a hyphen (-) to be consistent with the CSV Books and CSV Newspapers toolchains.

But as you can see, I didn't update any of the example instances. Will do that.

mjordan commented 6 years ago

@bondjimbond take a look now. Feel free to update any I have missed.