Download & Extend
Version Control API » Issues

Make repository URL backends pluggable

Posted by jpetso on January 5, 2009 at 12:12pm
Project:Version Control API
Version:6.x-2.x-dev
Component:API module
Category:feature request
Priority:major
Assigned:marvil07
Status:closed (fixed)
Issue tags:git phase 2, git sprint 8

Issue Summary

Repository URL backends provide the URLs for various repository related links. For example, one could select a WebSVN URL backend, enter the WebSVN root, and get all the URLs for free without having to enter them manually. Various URL backends might also provide URLs that can't be created using the default URL backend's simple string replacement mechanism.

At least, that's the theory, because the default URL backend ('versioncontrol_default_urls') is currently hardcoded. We might want to make URL backends actually pluggable sometime, although it doesn't seem like a big priority right now.

Login or register to post comments

Comments

#1

Posted by marvil07 on July 21, 2010 at 5:29am
Title:Make repository URL backends pluggable» Implement some url input helper
Version:5.x-2.x-dev» 6.x-1.0-rc2

After analysing it and talking with @jpetso, we found that pluggable url backends do not have enough reasons to be.

So at oop branch the idea of url backends dissappear.

In the other side, the functionality mentioned here is a helper, and it can be done with form alter, so it would be handy to have at least one as a reference implementation.

#2

Posted by marvil07 on February 10, 2010 at 8:43am
Version:6.x-1.0-rc2» 6.x-2.x-dev

#3

Posted by marvil07 on July 21, 2010 at 5:28am

related to #156118: Automatically generate the value of 'Link to webcvs/viewcvs' field for a project

#4

Posted by marvil07 on November 29, 2010 at 5:56pm
Status:active» closed (duplicate)

Marking this as postponed, since IMHO this will be happening as part of converting VersioncontrolRepositoryUrlHandler into several ctools plugins \o/ #976148: Recover links to VCS web viewers and issue tracker at views

#5

Posted by marvil07 on November 29, 2010 at 6:07pm
Title:Implement some url input helper» Make repository URL backends pluggable
Priority:minor» normal
Status:closed (duplicate)» postponed

Ok, I am moving the title back to the original, since the idea of making them pluggable come up again.

The main difference will be the approach, as mentioned in #4, but postponing this until the first ctools patch gets in: probably #979040: Make pluggable the process of mapping of raw vcs data to Drupal users

#6

Posted by marvil07 on December 3, 2010 at 1:36am
Status:postponed» active

reopening, since #979040: Make pluggable the process of mapping of raw vcs data to Drupal users is committed

#7

Posted by marvil07 on December 6, 2010 at 7:13pm
Assigned to:Anonymous» marvil07
Issue tags:+git sprint 6

have an uncompleted patch, going to post it here soon.

#8

Posted by marvil07 on December 6, 2010 at 9:06pm
Issue tags:-git sprint 6

untagging as decided today on meeting

#9

Posted by eliza411 on January 2, 2011 at 9:52pm
Priority:normal» major
Issue tags:+git sprint 8

Tagging for Git Sprint 8
Connects to the repo viewer ... #939024: Decide if we should switch to a homemade git viewer instead of cgit or gitweb

#10

Posted by marvil07 on January 6, 2011 at 10:37am
Status:active» needs work

Here is a patch with the general idea, it still needs some work, but uploading it since I will not be working on it for a day.

AttachmentSizeStatusTest resultOperations
pluggable-urls-v0.patch23.61 KBTest request sentNoneView details

#11

Posted by sdboyer on January 10, 2011 at 6:52am

Sorry my review of this got delayed.

  • There's a VersioncontrolRepository::$pluginInstances property that should probably be where the instantiated viewer object gets stored, for consistency with the way other plugins operate.
  • Let's move VersioncontrolRepositoryUrlHandler into its own inc file somewhere in a way more consistent with how other comparable classes are handled.
  • I don't really like mixing in links to the commitlog here, it situates these plugins in a position awkwardly halfway between encapsulating a separate repo-browsing application entirely and log viewer functionality, which (as part of VCAPI core) has other ways to be switched around. I'd prefer to simply get rid of any methods that deal with the commit log. I guess that basically means linking to a tree browser, an individual file view, and maybe a diff view.
  • There are some var_dump()s and dpm()s in the views handler.
  • If we want to keep consistent with the interface we've been using in the repo edit form, then we may need to add in some ctools modals to manage additional configuration of the url viewer plugins. Maybe. Depends on how much conf they need, versus how much is just hard-coded into the plugin.

So apart from wanting to decouple the commitlog from the repo browser in this plugin, looks like we're pretty well on the same page. Let's see some more patches :)

#12

Posted by marvil07 on January 12, 2011 at 5:21am
Status:needs work» needs review

Ok, I think this is ready.

What is new in this patch:

  • hook_update_N() to update $repository->data['versioncontrol'].
    • Remove url_handler object.
    • Add base_url string.
  • Move webviewer_url_handlers plugin documentation to an empty plugin.
  • Remove tracker url from url handler templates.

    • It is going to be re-added as a views handler option for operation
    messages.

  • Add vcs to filter webviewers by vcs.
  • Add %repo_name placeholder for URL templates at URL handler plugins.
  • Use $repo->pluginInstances data member to store instantiated object for webviewer.
  • Move VersioncontrolRepositoryUrlHandler into its own inc file.
  • Add directory_log_view URL template for URL handlers
AttachmentSizeStatusTest resultOperations
pluggable-urls-v1.patch32.23 KBTest request sentNoneView details
pluggable-urls-v0-to-v1.patch26.96 KBTest request sentNoneView details

#13

Posted by sdboyer on January 12, 2011 at 6:49am

I'll try to get to this a little later tonight, very focused on the beanstalkd stuff atm.

#14

Posted by marvil07 on January 12, 2011 at 3:54pm

I noticed some mistakes while implementing integration on views handlers, so here the fixes.

AttachmentSizeStatusTest resultOperations
pluggable-urls-v2.patch31.53 KBTest request sentNoneView details
pluggable-urls-v1-to-v2.patch2.89 KBTest request sentNoneView details

#15

Posted by sdboyer on January 13, 2011 at 3:18am
Status:needs review» needs work

+      if (!class_exists($class_name)) {
+        throw new Exception("Plugin 'webviewer_url_handler' of type 'webviewer_url_handlers' does not contain a valid class name in handler slot 'handler'", E_WARNING);
+        return FALSE;
+      }

There's static text that should be variable in those error messages.

+    unset($data['versioncontrol']['url_handler']);
+    $data['versioncontrol']['base_url'] = '';
+    $data = serialize($row->data);

I don't like the namespacing within the $data array, never did (sign of bad architecture, IMO). Let's not introduce any more of it.

The gitweb inc is still in there, and...

@@ -149,9 +149,14 @@ abstract class VersioncontrolTestCase extends DrupalWebTestCase {
        'auth_handler' => 'ffa',
        'author_mapper' => 'simple_mail',
        'committer_mapper' => 'simple_mail',
+      //FIXME use a general commitlog url handler
+      'webviewer_url_handler' => 'gitweb',
      );

...still referenced in the views handler.

I must admit, I'd never quite thought to do what you've done with the 'none' plugin for this - use variables statically declared as part of the plugin as args to the plugin's class. Kinda awkward, but OK for us to start with, anyway.

Apart from that stuff, looking pretty good. Fix it up and I think we're RTBC, though we'll need a follow-up issue to do the gitweb plugin in vc_git.

#16

Posted by marvil07 on January 13, 2011 at 4:09am
Status:needs work» needs review

Ok, here the changes mentioned on #15.

AttachmentSizeStatusTest resultOperations
pluggable-urls-v3.patch31.63 KBTest request sentNoneView details
pluggable-urls-v2-to-v3.patch3.78 KBTest request sentNoneView details

#17

Posted by sdboyer on January 13, 2011 at 6:50am
Status:needs review» fixed

OK great. Go ahead and commit.

#18

Posted by sdboyer on January 13, 2011 at 6:50am
Status:fixed» reviewed & tested by the community

#19

Posted by marvil07 on January 13, 2011 at 3:24pm

I changed some things before committing:

AttachmentSizeStatusTest resultOperations
pluggable-urls-v3-to-v4.patch6.96 KBTest request sentNoneView details
pluggable-urls-v4.patch30.9 KBTest request sentNoneView details

#20

Posted by marvil07 on January 13, 2011 at 3:39pm
Status:reviewed & tested by the community» fixed

Ok, it's in!

#21

Posted by System Message on January 27, 2011 at 3:40pm
Status:fixed» closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.