Closed (fixed)
Project:
Version Control API
Version:
6.x-2.x-dev
Component:
API module
Priority:
Normal
Category:
Feature request
Assigned:
Issue tags:
Reporter:
Created:
14 Jan 2011 at 19:32 UTC
Updated:
15 Apr 2014 at 22:25 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #1
marvil07 commentedOk, here a patch that:
Comment #2
marvil07 commentedSeems like versioncontrol_project can use this :-)
Comment #3
marvil07 commentedAdding an UI per backend per plugin, and hopefully a way to define default settings inside the plugin.
Comment #4
marvil07 commentedOk, finally the new patch!
what's new:
Comment #5
sdboyer commentedOverall, I think this is great. We really do need this; even if it's mostly in development environments (as opposed to real prod sites) where we get into funny inconsistent states where plugins aren't set that should be, plugins are now sufficiently essential to the basic functionality of vcapi that it's really not prudent to allow hard failures to occur when a plugin can't be found.
Good call on the 'None' mapper, that's _clearly_ the sensible default.
I'm a bit hesitant about the approach of having these global defaults for the base URLs of the webviewers. It feels like not-the-best architecture (really something that I'd rather encapsulate on the plugin), but I don't have any better ideas in mind and it clearly works, so it's not a blocking concern.
db_query('UPDATE {versioncontrol_repositories} SET data = %s WHERE repo_id = %d', $row->data, $row->repo_id);Gotta keep an eye on this, you're not quoting the string placeholder in both 6312 and 6314. I missed this when reviewing the patch containing 6311 and committed it unintentionally, and had to fix it - unquoted serialized strings always ruin queries.
Only other note is that I think it might be worth it to create an includes/plugins.inc file, and move some of our plugin code into there. If we add a ctools_include('plugins') at the top of such a file, then that could simply it, too - just switch most existing calls to ctools_include('plugins') over to ctools_include('plugins', 'versioncontrol').
Comment #6
sdboyer commentedI skipped creating the separate plugin file (no time for that noise right now), but fixed the db problems (one was actually already committed), made some other minor tweaks, and committed. Sorry marvil07, we just really need this one in and can't wait.