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

Decomposing blacklight generator #600

Closed
wants to merge 1 commit into from
Closed

Decomposing blacklight generator #600

wants to merge 1 commit into from

Conversation

jeremyf
Copy link
Contributor

@jeremyf jeremyf commented Aug 23, 2013

Created a blacklight:models generator that is now used by the
blacklight generator.

Created a blacklight:models generator that is now used by the
blacklight generator.
@cbeer
Copy link
Member

cbeer commented Aug 23, 2013

I'm not sure I know why some generators are in one place and not the other. I'll add some inline comments asking for clarification.

EDIT: Ok, got it. I guess it makes sense, after resolving that one config question.

def create_configuration_files
copy_file "config/solr.yml", "config/solr.yml"
copy_file "config/jetty.yml", "config/jetty.yml"
directory("config/SolrMarc")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we should split the config/SolrMarc out, and push it back into the catch-all generator?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, the config/SolrMarc step should be coupled with the lib/solr_marc.jar step. They don't make sense independently.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's probably a bug that we are generating solr_marc.jar into lib/ in the first place.

It means that if Blacklight updates it's bundled version of solr_marc.jar, and you update Blacklight in your app -- you'll nevertheless still be using the solr_marc.jar that was generated into your app.

When I wrote the solr_marc.jar stuff, the intention was that ordinarily an app would use a solr_marc.jar directly from the gem, so if the gem updated it, the app would be using the new version. Optionally, if you wanted to use a solr_marc.jar differnet from the one bundled with Blacklight, you could put it at 'lib/solr_marc.jar` in your local app -- say if you needed to use a newer version than the bundled one, or one compiled with a bug fix that wasn't in the bundled one yet.

And it used to work that way.

At some point after that, literally a couple years ago now, someone misunderstood the intent and changed the generator to always generate a solr_marc.jar into your local app.

I've mentioned it several times, but havne't had time myself to actually figure out what's going on and fix it, and apparently nobody else has had the motivation either.

But if you are working on this stuff at all, please consider making it no longer generate solr_marc.jar into the local app, at least not as a default. That was not the intention, and is harmful, as it will keep many local apps on old buggy versions of SolrMarc, even when new versions have been released and bundled with the relevant gems and the app has updated it's gems etc.

@cbeer
Copy link
Member

cbeer commented Aug 23, 2013

I'm going to refactor the MARC stuff into its own generator, and replace this PR.

@jeremyf
Copy link
Contributor Author

jeremyf commented Aug 23, 2013

@cbeer 👍

@cbeer
Copy link
Member

cbeer commented Aug 23, 2013

Merged in dea86f4. Thanks @jeremyf .

@cbeer cbeer closed this Aug 23, 2013
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.

None yet

4 participants