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

SolrJsonWriter, guard marc-specific in record_id_from_context fixes #92 #94

Merged
merged 2 commits into from
Feb 20, 2015

Conversation

jrochkind
Copy link
Contributor

For now I think this is all we need for #92. Although it really needs a test, since I'm not sure how to use Indexer with non-MARC at all at present, I'm not sure how to write one!

Long-term, I think the right design choice is capability to specify in configuration either a Traject::Indexer sub-class and/or one or more mix-in modules to the Traject::Indexer. The primary use case for that would be different source record types -- which would over-ride methods like record_id_from_context in source-type-specific ways but also likely add other macro methods (eg not including not extract_marc but including other source-type-specific useful macros -- I tried to name all the marc-specific macros with "marc" in them for this reason.

Once we went down this path, there'd be a generic Indexer base class that did not have any marc specific stuff (including not mixing in the MARC-specific macros) -- but I'd still want the marc-specific one to be default probably, but that could be debated at the time.

I think it's too early for that now, before we have cowpaths to pave, and I think this very simple change should make it possible for erikhatcher to do the experimentation he wants? @erikhatcher?

@jrochkind jrochkind force-pushed the json_writer_marc_guard branch from b034fd6 to 89c9feb Compare February 20, 2015 17:51
@billdueber
Copy link
Member

Added check to make sure there's a 001.

billdueber added a commit that referenced this pull request Feb 20, 2015
SolrJsonWriter, guard marc-specific in record_id_from_context fixes #92. Gonna merge this is and throw out a patch release.
@billdueber billdueber merged commit fd0887e into master Feb 20, 2015
@jrochkind
Copy link
Contributor Author

good call, thanks!

@billdueber billdueber deleted the json_writer_marc_guard branch January 31, 2016 02:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants