I would be great if we could ignore specific folders/patterns when using the --git-check option, helping developers to actually push repositories they should while ignoring contrib modules/themes etc.

I have in the patch added an extra option called --git-check-ignores which takes a grep based regex for ignoring parts of the find result.

Example usage:
drush situs-build --root=htdocs --make-file=./platform.make --git-check --git-check-ignores=/contrib/

CommentFileSizeAuthor
#9 drush_situs-git-check-ignore-2242071-9.patch5.36 KBlslinnet
#3 drush_situs-git-check-ignore-2242071-3.patch5.35 KBxen
#2 add-git-check-ignore-options-with-drush-scan-directory.patch2.7 KBlslinnet
add-git-check-ignore-option.patch1.21 KBlslinnet

Comments

xen’s picture

xen commented
Status: Needs review » Needs work

I can see how this is needed when checking specific revisions of contrib out and patching them. But I have a few issues with the implementation.

For starters, using find was a quick hack that I should've fixed up long ago to use drush_scan_dir. So rather than passing the find output through grep, the filtering should be done inside the loop.

And as much as I love regexps, they can be quite the loaded gun for the inexperienced, so I would prefer to have two options: --git-check-ignore and -git-check-ignore-regexp, the first just taking fixed strings. By using drush_get_option_list(), they'll be able to take a comma separated list of strings/regexps.

In general, I don't think it's very good DX that each dev will have to remember which ignores to add, but this will serve as the groundwork for something more handy. I think that it's possible to add "situs[] = stuff" to the makefile and have Drush make quietly ignore it. Then the ignore commands can be specified there, and situs itself would copy the options over.

lslinnet’s picture

lslinnet commented
Status: Needs work » Needs review
StatusFileSize
new add-git-check-ignore-options-with-drush-scan-directory.patch2.7 KB

I am not quite sure if I actually like this approach, it seems quite dirty to me - but after some fiddling around I have gotten to a point where this would work.

Having each developer memorise the parameters does indeed seems like a little too much, but it can be used with drushrc.php where you specify some generic ignore patterns.
using the project pattern from lullabot's drupal boilerplate (https://github.com/Lullabot/drupal-boilerplate) it allows you to set default options for each drush command in the project specific drushrc.php file - we are using something similar hence the request for this functionality.

--git-check-ignore=htdocs/profiles/myprofile/libraries/flexslider,htdocs/profiles/myprofile/themes/custom/myprofile_theme
--git-check-ignore-regex=/global/,/contrib/

Notice that the user has to correctly escape / in the git-check-ignore-regex's so if /theme/mothership/ becomes /theme\\/mothership/ - have opted away from trying to figure out what should be escaped and what shouldn't in the regexes

Also there might be an issue if you wish to do an escape based on "," as i am just using a simple explode to separate the regexes - alternatively we could change the regexes to be just one complex regex?

xen’s picture

xen commented
StatusFileSize
new drush_situs-git-check-ignore-2242071-3.patch5.35 KB

Tests broke (I should fix them up so they can run on Drush > 5.10).

Added new tests and ended up rewriting most of the patch. Please check whether it works for you.

lslinnet’s picture

lslinnet commented

Tested and works as expected, quite a bit more clean code!

How ever shouldn't the 

$path_ignores = implode('|', $path_ignores);

be

 $path_ignores = implode('$|^', $path_ignores);

so that each path gets start of and end of string checking?
e.g
--git-check-ignore=htdocs/profiles/myprofile/libraries/flexslider,htdocs/profiles/myprofile/themes/custom/myprofile_theme
would be the same as
--git-check-ignore=htdocs/profiles/myprofile/libraries/flexslider,myprofile_theme
with the current structure.

can see I should read up on preg regexes patterns wasn't aware of the preg_match :)

lslinnet’s picture

lslinnet commented
Status: Needs review » Needs work

Forgot to change status.

lslinnet’s picture

lslinnet commented
Status: Needs work » Needs review
StatusFileSize
new drush_situs-git-check-ignore-2242071-6.patch5.36 KB

And uploaded patch with the change.

xen’s picture

xen commented

No, I was sleepy. What I wanted to do was this:

if (($path_ignores && preg_match('{^(' . $path_ignores . ')$}', $repo)) ||
($regex_ignores && preg_match('{(' . $regex_ignores . ')}', $repo))) {
continue;
}

See if drush_situs-git-check-ignore-2242071-3.patch doesn't work with that.

lslinnet’s picture

lslinnet commented
Status: Needs review » Reviewed & tested by the community

yes that works too, can't find anything that should hold it back from being committed, moved to RTBC.

lslinnet’s picture

lslinnet commented
StatusFileSize
new drush_situs-git-check-ignore-2242071-9.patch5.36 KB

Ahh you deleted the patch file :) here is one with the latest change.

  • Commit 1f2bd0f on 7.x-1.x by Xen: 
    Issue #2242071 by lslinnet, Xen: Allow git repositories in specific...
xen’s picture

xen commented
Status: Reviewed & tested by the community » Closed (fixed)