This patch is for the Drupal.org D7 upgrade. I can of course implement the mapping in a custom module, but to me it makes sense to offer a mapping as part of the machine_name module.
For sites that don't use Solr, the hook does nothing.
| Comment | File | Size | Author |
|---|---|---|---|
| #8 | 1822252.patch | 658 bytes | grendzy |
| #1 | 1822252.patch | 789 bytes | grendzy |
| #1 | 1822252.png | 25.06 KB | grendzy |
Comments
Comment #1
grendzy commented/node/N/devel/apachesolr after applying the patch:
Comment #2
senpai commentedElevating to critical because we need this for D7 launch.
Comment #3
dwwI don't know enough about the innards of the Solr API to give this a meaningful review, but Senpai is asking for one, anyway. ;)
I guess this looks reasonable. I was confused as to how this hook knows what field names to operate on, but grendzy assured me it's operating on field types, not field names.
I can't RTBC this with a clear conscience not knowing the innards of Solr, but I don't see any obvious reason to set this to 'needs work', either...
Cheers,
-Derek
Comment #4
jhedstromThis looks reasonable to me. I haven't tested it though.
Comment #5
c-logemannIt's difficult for me to test this patch. I need a RTBC or better an explicit OK of the "Drupal.org D7 upgrade initiative" for the special stable release:
#1647904: Machine Name needs a 1.0 stable release as part of the Drupal.org D7 Upgrade Initiative
Comment #6
senpai commentedComment #7
csevb10 commentedOk, in general, this is basically fine.
I don't think all the array elements are technically necessary or accurate, but they don't really cause an issue:
necessary/fine/agree:
- 'indexing_callback' => 'apachesolr_fields_default_indexing_callback',
- 'index_type' => 'string',
- 'facets' => FALSE,
- 'dependency plugins' => array('bundle', 'role'),
- 'multiple' => FALSE,
not sure these are necessary:
- 'map callback' => '', // default setting, except using FALSE
- 'facet mincount allowed' => FALSE, // default setting
- 'hierarchy callback' => FALSE, // default setting
- 'name_callback' => '', // not actually a key 'name callback' is, but don't need it here
Comment #8
grendzy commentedThanks for the feedback, here's a re-roll.
Comment #9
grendzy commentedIf anyone's wondering specifically how this will be used on Drupal.org, the project node's short name was previously stored under
ss_project_uri, and pulled bysecuritydrupalorg_sync_projects(). The new machine_name field replaces this of course, so with the patch Solr will map fields of this type toss_[FIELD-NAME].Although the patch was motivated by the d.o upgrade, it's potentially useful to anyone who uses Solr.
Comment #10
senpai commented@grendzy, thanks for the re-roll. This one looks RTBC.
Comment #11
c-logemannCommitted: http://drupalcode.org/project/machine_name.git/commit/93127d609b27c447c5...