Disable makefile by default

I was planning on adding this I just haven't had the chance. Thanks for opening this.

You can also automatically download the json2 library, which is required for jQuery Cycle custom options:

libraries[json2][download][type] = file
libraries[json2][download][url] = https://raw.github.com/douglascrockford/JSON-js/master/json2.js

The current practice is to add an "example" makefile. According patch attached.

Thanks Simon Georges, Cyberwolf, redndahead, heshan.lk
commit:b9a04d9

By making it a makefile, you're "forcing" the download of the libraries when using Views Slideshow inside a makefile (make is recursive). Naming it with a .example prevents this from happening, by design, and I think that's the recommended way to do it (so users are still free to download from another location, or manually build their own makefile).

I'm ok with it not having .example on the end since there are ways to override the make file. i.e. putting the override sufficiently above your call to views slideshow. But I could see it from both sides.

While the use of makefiles has to be encouraged, like Simon Georges, I think it would definitely be a better option to rename it so that it won't get recursively interpreted by another makefile downloading Views Slideshow. It interferes with existing build processes; in my particular case libraries end up being dowloaded in a weird 1/libraries folder instead of libraries.

How about following the general trend and renaming it to views_slideshow.make.example? This is what Colorbox, D3.js and Leaflet do, for instance: see this commit to Leaflet, which addresses this (unwanted) recursion issue.

We should probably take a good look at this. I too am finding that I have issues where having a makefile included in a contrib module, by default (and that is NOT renamed with a .example suffix), is causing significant issues with my project's main site makefile.

My makefile directs Drush to stick all contributed libraries, modules, and themes into a "contrib" subdirectory alongside my "custom" subdirectories within each of the three aforementioned parent directories.

My various attempts at overriding view_slideshow's makefile to stick json2 and jquery.cycle into my "contrib" libraries subdirectory only result in getting duplicate copies of these libraries, a set in my contrib folder, and a set in the parent library folder.

Manually renaming view_slideshows makefile with a .example suffix will not work because I'm automatically pulling these files using my makefile, which goes right on to install those libraries using the module's included makefile.

I think the best solution in this case, and probably most others, is to set view_slideshow's makefile to be "off" by adding ".example" suffix to the .make file. This is now the recommended practice as you'll find in various other modules such as ckeditor, geshifilter, mailchimp, and others I may have not yet seen.

I'll add more if I find out more :)

This thread speaks of setting some overrides at the top of the parent makefile: https://drupal.org/comment/7072746#comment-7072746

I would like to suggest keeping our library dependency and download within own .make for simplified management, and at least this works well for DruStack for quite a long time.

Patch for downloading entire package from github, port from drustack_extra.make

Patch now applied to drustack_extra.make

hswong3i: I'm afraid I don't understand your approach, neither do I see why it would be necessary to download the whole library. Could you please be more specific, what issue were you trying to address?

sorry, dbl post

I'm using a similar workaround as hswong3i, where *my* drush makefile will patch view_slideshow's makefile -- commenting out the libraries so they won't auto-install outside by "libraries/contrib" folder or overwrite what I've already pulled in.

Edit: looks like Drush will not patch makefiles, instead it just throws an error. Looks like the only way to go here is to just not include these libraries in the makefile, or have no makefile at all in this module.

I also think a makefile shouldn't be forced on users by default, especially with Views Slideshow as a lot of users will now be wanting to use the new Cycle2 library instead...

Here's a patch that renames the makefile so it's not used by default.

Why is the option in #10 not ok?

That option seems to me like a hack...

I have, however, posted a question on Drupal Answers asking what the recommendation is for this type of situation: http://drupal.stackexchange.com/questions/103538/standards-recommendatio...

I agree, this would seem better to me than what I referenced in #10.

The accepted practice for contrib make files is to add ".example" to the end of the filename. BWPanda's patch in #16 is the correct fix for this issue. The patch applies cleanly to 7.x-3.x. Setting to RTBC. Please commit and cut a new release as soon as possible. Thanks.

For reference, the Rules module went through the exact same problem when adding a make file. It ended up breaking existing build processes. In the end, ".example" was added to the end of the filename. #1329346: Remove Drush Make dependency definitions (rules.make)

I think a make.example is useless. It's better in a readme. I actually don't see a problem with the .make, but if one of the other maintainers want to remove it or rename it feel free.

@redndahead I do not think it should be removed. I think it should stay. This way you are also able to test the module on simpletest. I think this should be closed as works as designed.

Isolesen: did you read the whole discussion? Shipping a makefile with a bare .make extension with the module really, really, badly breaks automated builds (I am stuck with version 3.0). The makefile should certainly stay, but with a different extension. This way you are still free to use it no matter what the extension is, and it will not get recursively interpreted by drush make, playing havoc with the rest of one's files, for instance when you are building a platform from an installation profile.

FMB: You didn't read the part where I said you can override this in your own make file.

https://drupal.org/node/1330166#comment-7859729

It's not hard. I do it on my installation profile. Works great.

redndahead: I did try it back then, but could not make it work for my setup. Besides, I do not think it is up to the user to counter the effects of an unwanted makefile.

Breaking existing build processes by default is very different than "override this in your own make file". Other high-profile contrib modules have already run into this exact same issue and resolved it by either removing the offending make file or renaming it to .make.example #1329346: Remove Drush Make dependency definitions (rules.make)

I think it's a bad practice so I'm not going to fix it. If the other maintainers do it then that's fine.

redndahead, could you please elaborate on why the suggested fix is "bad practice"? Thanks in advance.

Because the point of the make file in a project is to provide what is required for that module. There are options to override this requirement if you need something different, but as an install profile maintainer I don't need to know what your requirements are just that they are fulfilled when I add your module to my install profile. Also as lsolesen pointed out there is a awesome resource in simplytest.me that just works better when you have a make file. By having an example file you've might as well have put that inside the readme file.

There are options to override this requirement if you need something different

Could you please elaborate (code snipped) because, as @FMB, I did not succeed is writing a profile .make which would override what your module .make is trying to do (and I end up with a 1/libraries folder too).

+1 for the patch in #16

As mentioned above, I posted a question on Drupal Answers asking about the recommended approach to module's makefiles.

@jonhattan replied and confirmed that

Modules should not include a module_name.make file, to prevent drush make running them on recursion. Adding module_name.make.example is desired...

Committed to http://drupalcode.org/project/views_slideshow.git/commit/b54d6d5.

@BWPanda That is the opinion of @jonhattan. If I had replied the answer would have been different. I agree with @redndahead that this should have been kept as is. Also this commit breaks stuff for people already relying on the make file being included in the module.

That is the opinion of @jonhattan.

Indeed. The point of the DA question was to ascertain the standard, recommended practice for Drush Make files. @jhedstrom's previous recommendation was backed up by @jonhattan and, as they are both prominent Drush contributors, I value their opinions over yours (with all due respect). If they had suggested instead, as you do, to name Make files as normal then I would go along with that and this issue would be moot.

...this commit breaks stuff for people already relying on the make file being included...

This is a result of doing things in a non-standard way and then changing it to be standards-compliant. The maintainers will just have to decide whether to allow the change and include a notice in the release notes, or revert to the non-standard way just for the sake of those people.