Fixed Issue #21 : #64
Conversation
Added master branch zip download link for addons if hosted on github.
|
Fixes Issue #21 |
|
Great pull request, I would like to add something in to make it more awesome for a certain group of users. My thoughts are that the download should focus on the latest stable version if possible and default to master if there is no packagist friendly version numbered stable version available? My thinking is that the people most likely to use the download button just want to get a zip file and upload or drop it into their webroot. This is a fair assumption I believe as a more seasoned SilverStripe developer isn't likely to do this (they would use composer if they needed to grab dev-master branch for contributing or simply do a git clone). This might also be an incentive for people to start getting better with remembering to do number versioned releases of modules. To write this as a user story... As a new comer to SilverStripe module installation (that hasn't learnt composer yet), I want to download a stable module package so that I can use it in my project with confidence that the code is in a known state. |
| $repositoryLink = str_replace('git@github.com:', '', $this->Repository); | ||
| $repositoryLink = str_replace('.git', '', $repositoryLink); | ||
|
|
||
| $repositoryLink = 'https://github.com/' . $repositoryLink ; |
There was a problem hiding this comment.
What if it's already a GitHub URL? Or is a git:// one?
|
Good points @simonwelsh :) @vikas-srivastava fancy looking into these points and updating your pull request? 👍 |
|
@camfindlay Thanks for pointing that. I will update my pull request. Thanks @simonwelsh I will look into those things soon. At-least we are talking on this issue ;) |
|
Hmm, been thinking about this a bit more since I've written #21. The archive will have the wrong folder name, which means any paths to assets within the module folder won't work. For example, the blog module extracts into Do you maybe want to have a think how we can tackle this through documentation alongside the download link? We could replicate the logic in https://github.com/composer/installers/blob/master/src/Composer/Installers/BaseInstaller.php#L32 to determine the install path of a module? |
|
This has probably been addressed elsewhere, But this could cause trouble for modules that have requirements specified in My modules are generally meant to be installed via composer, and not downloaded as a zip. They can be installed via zip, but they won't work until you also manually install each dependency (and the correct versions). Maybe this could be addressed, by saying more than "We Recommend using composer to install Add-ons, but if you don't want to you can download a zipfile"? Maybe we could use the |
|
thanks @camspiers yes I agree. We should at least just give them the information that downloading this code will be missing dependancies. Maybe a link to learn composer? This does give some motivation for the user to learn composer... but if they just want to grab the code they still can. |
|
Given there's general concerns with showing downloads (unclear how dependencies and installation are handled), and there's specific concerns with the implementation (raised by Simon), I'm going to close this pull request for now. Even if we add download instructions mentioning dependencies, I'm not really happy to straight up link to the github downloads, since they install into a wrong folder and will break most modules. So looks like we'll need to create our own ZIP archives, which is a different piece of work altogether and much more complex. @vikas-srivastava Maybe you could have a think on how to do this implementation? ZIP creations will have to be processed in the background of course. |
Added master branch zip download link for addons if hosted on github.