Proposal: Improved file collection

cha0s - June 22, 2009 - 19:47
Project:Import / Export API
Version:6.x-1.x-dev
Component:Code
Category:feature request
Priority:normal
Assigned:cha0s
Status:needs work
Description

ImportExport API is a little rough around the edges when it comes to user-defined engines, definitions and fields. This is because it's hardcoded in places to only search for those in its own location, instead of looking in another modules directory. This can be likened to views, where it would be silly to expect another contrib to inject its files into views' directory.

This patch changes the file naming structure a bit. Instead of just having importexportapi_SOMEFILE.inc for engines, fields, and definitions, we change the file signature a bit for each, to eliminate ambiguity. This allows us to efficiently and automatically collect the files needed.

The new naming structure is as follows:

Fields: importexportapi_field_FIELD.inc
Engines: importexportapi_engine_ENGINE_CALLBACK.inc
Definitions: importexportapi_definition_MODULE.inc

I couldn't figure out how to make the patch automatically handle the filesystem changes, so I included the output from stderr which gives a good idea of what changed and where.

Thanks for reading, this will be a great improvement to importexportapi's extensibility if/when implemented.

AttachmentSize
importexportapi.file_handling.patch9.73 KB

#1

cha0s - June 27, 2009 - 07:55

Hey, forgot I missed the engine include name changes in the last patch.

P.S. Anyone home..?

AttachmentSize
importexportapi.file_collection.6.x.patch 9.22 KB

#2

earnie - June 27, 2009 - 18:00

I've had a sad last five weeks with dire family emergencies and a two week civic duty. On top of that my system hung up the email and I wasn't getting many delivered to me including your original post. I've been heavily involved with getting out a working beta version for xmlsitemap. I'll take a look at your patch ASAP; I'm trying to catch up on a lot of things. I do have a priority for getting this module improved and will eventually get to working on it. Thanks for the patch.

#3

cha0s - July 7, 2009 - 07:08

I'm going to commit this if no one has objections.

#4

earnie - July 7, 2009 - 11:55

How can you commit it, you don't have write access do you?

These are hefty changes to the structure of the module. I need review time. Jaza what do you think?

#5

cha0s - July 7, 2009 - 17:44

I do. I have been trying to let my changes be vetted.

I'm not just going to let my patches sit in purgatory forever though. :) If you want a little longer to do a review, fair enough.

#6

cha0s - September 8, 2009 - 15:54

Bump.

#7

earnie - September 9, 2009 - 13:19

Give me until this Monday at 19:00 UTC.

#8

earnie - September 14, 2009 - 12:49
Version:HEAD» 6.x-1.x-dev
Status:needs review» needs work

I like what you are trying to do but I think we need to put these files in an importexportapi directory. Assuming module named foo we would then have modules/foo/importexportapi/fields_myfield.inc or modules/foo/importexportapi/engine_myengine.inc, etc. The patch also needs to provide an update path but we can work that out once we have the primary change down pat.

#9

cha0s - September 15, 2009 - 19:02

I waited months for a review and you didn't even explain why that should be done. ;) What's the reason we should require that?

#10

earnie - September 16, 2009 - 13:07

The reason is that it keeps the top module directory clean by putting the files relevant to importexportapi in its own directory.

 
 

Drupal is a registered trademark of Dries Buytaert.