/vendor & /tools cleanup

[jakecleary]

This should be rebased onto #14 and #15 so don't merge this yet!

[thekabal]

I'm sorry to say this patch (moving the tools to the vendor path) is unlikely to complete.

I tested it manually, and found many problems. The first and foremost is that instead of "just" the phar archive for the specific tools, composer/packagist wants to install a pile of unneccesary dependencies for almost every package. Prior to the change, the directory /tools is 6.8mb, and the vendor directory is 21mb.

After the change, the vendor directory is 43mb - almost half of the (then) 109mb total distribution, and a substantial increase in size for the same functionality in different places.

Also, composer reports that "sebastian/phpdcd is abandoned, you should avoid using it. No replacement was suggested."

None of this addresses the eight additional dependencies that it would have liked to pull in to meet these requirements.

I'm going to say no. Moving them out of tools wasn't a terrible idea, but the devil is in the details, and doubling the archive size is a hefty price to pay to have the tools loading from elsewhere.

[jakecleary]

Hey @thekabal,

Tools

If you'd like to keep them as version-controlled dependencies that's totally fine!

However, I would like to stress that these are development dependencies, so install size doesn't really matter. I can't imagine these being bundled in release archives as they're only really for use by developers on their local machine. Also, they come with the added benefit of being more easily updatable.

Regarding sebastian/phpdcd, it's simply a warning to say it's not being actively worked on anymore, which is true regardless of whether we use the phar or packagist version. The package itself can't be deleted from packagist so we don't have to worry about it disappearing.

Vendor

I really think we should at least take these out of version control. It massively bloats the repo and the whole point of using something like composer is that it manages the dependencies for you, not the repository. There's not point tracking changes to dependencies as they're already in git, owned by the package maintainer. When it comes to public releases we can build a nice script that pulls them all in and packages it all together; it's not too difficult.

Let me know what you think 😄 Whatever the outcome, I respect your decision!

[thekabal]

I've gone ahead and migrated the tools -> vendor/bin, and used composer to pull them in. I don't love the bloat, but it is more maintainable and in line with correct development processes. Also, as you say, it is development, not prod, so hopefully the impact will be minimal.

"When it comes to public releases we can build a nice script that pulls them all in and packages it all together; it's not too difficult." I'd welcome further info on this comment. I'm tracking it for us at https://github.com/thekabal/tki/issues/51 , and https://github.com/composer/composer/issues/5440 indicates that it is a non-trivial problem to solve, and present for other teams.

[jakecleary]

Using the tools pulled in through composer is a step in the right direction.

The next step (in my opinion, although it is an industry standard) would be to stop tracking the vendor directory entirely, and rely on using composer install to pull in the correct dependencies.

Then, when you want to package the full production release, we'd need a script that clones the latest tagged release, runs composer install (so without the dev-dependencies/tools), and zips it all up.

I personally wouldn't worry too much about composer/composer#5440, as I believe that's more to do with the current situation with shared hosting limitations, and uploading zip files through a website's frontend, not via FTP. Bare in mind most people who want to host a copy of this game will upload it to their servers directly via FTP. I believe that issue you linked to is to do with plugin uploads through Joomla's plugin admin page, which is a different matter as php's max_filesize_upload has nothing to do with FTP uploads.

Let me know if I'm not being clear here!

[jakecleary]

@thekabal Any thoughts on removing the vendor directory from tracking?

[thekabal]

Its unlikely I'll change it until https://github.com/composer/composer/issues/5440 completes. Essentially, for shared hosts, we can't assume that the admins will have the opportunity to run command-line scripts (composer). As that is the case, we don't have a great alternative other than increasing the burden on the admins further to now include running composer locally (with all of the compatibility and documentation challenges that would also entail - Windows, OSX, Linux, installing and configuring PHP, composer, etc).

Those are all adding burdens to admins while gaining us very little other than a directory not being tracked. We've got many other challenges that are much more pressing and that add value.

But that said, if Composer adds support for running it from a web script, I wouldn't be opposed in theory.

[jakecleary]

Hi @thekabal, I think you're misunderstanding what composer/composer#5440 is about. It's to do with uploading plugins through the Joomla admin panel. This project wont be installed that way, it'll more than likely be via FTP directly to the server (or rsync, or any other file syncing tool).

The way to avoid people having to run composer manually before installing the tool is to have you supply a release build, which you could do by running composer yourself and zipping it all up, or using a very simple bash script to do it for you. You would then add the release build as a download link on this project's releases page (on github and/or the official website).

I'd be happy to provide a PR that does the following:

• Remove the vendor directory from being tracked
• Provide a very simple (for now) release script

It may seem like a pointless thing to prioritise but having a tidy/standards-compliant repo will help with attracting contributors, and generally make the project easier to manage as a developer. Also, I'm not sure what's wrong with having contributors devote time to small things that will make developers' lives easier while you work on the main issues, it's not slowing the project down.

Let me know what you think :)

[thekabal]

I'm not sure how you and I can disagree on that bug when the first sentence in the bug report is "Composer is intended to be executed from CLI, however sometimes it would be useful to actually run a composer install within a web page.", which is exactly what we need also - the ability to run composer from within a web page. While yes, the project will often be installed via FTP, having the ability to run composer from within a webpage would mean we don't need to ship/release/include the vendor directory at all. It would literally eliminate the need for BOTH a release script AND for having the vendor directory in repo.

Further, I mentioned previously that I have no issue with a release script that abstracts those needs away. The difference is between your perception/expectation of it being "simple for now", and my goal of not taking the shortcut of removing the vendor directory until/unless the release script is solid.

My suggestion to reach the goal of removing the vendor directory is for us to focus on building out a release script that while simple are also well tested, easy to use, and polished. A two-line bash script that runs composer and zips files isn't exactly compelling. Then the argument to switch becomes (in my mind) much better.

Personally, I don't see much value to it, so it isn't my itch to scratch. We have a LONG way to go to become more attractive to developers. You are welcome to develop a release script, and I'm happy to test it and merge it, or even give guidance on what I'd look for (does it work on most platforms, does it have clear debugging, is it simple to use, etc). I'm happy to have the help, and I've merged most of your contributions - I think you are helping quite a bit! I just want to see a better release script before making a substantial change at this point to the repo.

[thekabal]

Vendor has been removed from git, added to gitignore, and composer install will be run for releases. Many thanks for the suggestions, sorry it took a while to implement!