LibreHealthIO / lh-ehr

LibreHealth EHR - Free Open Source Electronic Health Records
Other
239 stars 264 forks source link

Composer files - clean up needed #725

Open tmccormi opened 7 years ago

tmccormi commented 7 years ago

Are these files needed? we are not "installing" any outside packages using composer builds in the current model to my knowledge.

./modules/html2pdf/composer.json ./modules/html2pdf/vendor/composer ./modules/html2pdf/vendor/tecnickcom/tcpdf/composer.json ./modules/html2pdf/vendor/setasign/fpdi/composer.json ./modules/html2pdf/composer.lock ./composer.json ./interface/tags_filters/composer.json ./interface/tags_filters/vendor/symfony/process/composer.json ./interface/tags_filters/vendor/composer ./interface/tags_filters/vendor/components/jquery/composer.json ./interface/tags_filters/vendor/kriswallsmith/assetic/composer.json ./interface/tags_filters/vendor/robloach/component-installer/composer.json ./interface/tags_filters/composer.lock ./library/vendor/composer ./library/adodb/composer.json ./assets/js/summernote-0-8-2/composer.json ./composer.lock

aethelwulffe commented 7 years ago

Nope.  I have had those files in a cut-list on several PR's in the past.  They seem to be everywhere.  Unfortunately, I recall there are some files named "composer" that were not necessarily composer.

The habit has been to install a whole complete package from whatever junked up set of source files are available, rather than cleaning it up and using only the required components.  Too much work I guess.

On 2017-10-15 13:36, Tony McCormick wrote:

Are these files needed? we are not "installing" any outside packages using composer builds in the current model to my knowledge.

./modules/html2pdf/composer.json ./modules/html2pdf/vendor/composer ./modules/html2pdf/vendor/tecnickcom/tcpdf/composer.json ./modules/html2pdf/vendor/setasign/fpdi/composer.json ./modules/html2pdf/composer.lock ./composer.json ./interface/tags_filters/composer.json ./interface/tags_filters/vendor/symfony/process/composer.json ./interface/tags_filters/vendor/composer ./interface/tags_filters/vendor/components/jquery/composer.json ./interface/tags_filters/vendor/kriswallsmith/assetic/composer.json ./interface/tags_filters/vendor/robloach/component-installer/composer.json ./interface/tags_filters/composer.lock ./library/vendor/composer ./library/adodb/composer.json ./assets/js/summernote-0-8-2/composer.json ./composer.lock

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/LibreHealthIO/LibreEHR/issues/725, or mute the thread https://github.com/notifications/unsubscribe-auth/AAhzF8E80v2BXTEeaT4Z_miYvQRuaTHsks5sskKDgaJpZM4P50xS.

kchapple commented 7 years ago

@tmccormi @aethelwulffe I would keep them, as they do not take up too much space, and do provide information about the versions of these components and their dependencies. And if we ever wanted to upgrade anything using composer we could.

aethelwulffe commented 7 years ago

Just my point of view:

Anything that is kept must be maintained, even if it is never used.

Ref: https://github.com/sensiolabs/security-checker

  I am not sure why one would include what are essentially makefiles in a deployed product.  The larger issue is that composer, being limited in how it manages file locations, should in fact be running on the development machine at a higher directory level than the webroot...not with the application leaving stuff essentially all over the place, and then there are little composer files everywhere in mixed directories.

I find that it supports monolithic architecture fine.  It does not specifically support any sort of package module scheme.  There are other limits that have demonstrated, to me, any net benefit is restricted to enterprise level operations that have single points of authority controlling the content.

Upgrading, using composer, seems to be very messy.  On your machine, space is not an issue.  For an unfunded distribution machine, it starts getting important.

Personally, I think we need to be picky and minimalist when choosing 3rd party assets.

I think that if an individual developer wants to utilize composer (which is not, and never will be deployed on anything but a few enterprise vendor machines, and only Linux: because on windows it I has its own prerequisite and permissions dependency hell) then they could and should utilize it to paste in the files they want, where they should actually go, then leave the actual application code base clean.  The composer configuration and dev toolset could, and should go into separate repo.

Just my opinion...

On 2017-10-19 09:41, Ken wrote:

@tmccormi https://github.com/tmccormi @aethelwulffe https://github.com/aethelwulffe I would keep them, as they do not take up too much space, and do provide information about the versions of these components and their dependencies. And if we ever wanted to upgrade anything using composer we could.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/LibreHealthIO/LibreEHR/issues/725#issuecomment-337911211, or mute the thread https://github.com/notifications/unsubscribe-auth/AAhzF6scTX9szPYVlw2B6YiLxhZidFVrks5st1F_gaJpZM4P50xS.

teryhill commented 7 years ago

those files could be moved to a place outside the code base as an archive for possible use later if they are needed.

kchapple commented 7 years ago

Since we're not using composer as a project at this time, most of these could be deleted. Except the root level one because it is actually used by the plugin system. It was my hope before the Tags/Filter thing was permanently checked into the repo, that it would be installed via composer. I would heavily support a move towards a single composer file and install dependencies via composer.

I am not sure why one would include what are essentially makefiles in a deployed product.

This is the source code repository, not the deployed product. In my 15 years as a professional developer, I've never seen makefiles outside of the source tree.

I find that it supports monolithic architecture fine. It does not specifically support any sort of package module scheme.

This is exactly what it does though not in the way we're using it. Perhaps we should move all of these composer packages to a single directory (like vendor) so they can be managed?

Upgrading, using composer, seems to be very messy.

I don't understand this at all. When used correctly, "composer update" updates all of your modules and dependencies. How is that messy?

Personally, I think we need to be picky and minimalist when choosing 3rd party assets.

I agree.

never will be deployed on anything but a few enterprise vendor machines, and only Linux

Composer is a PHP script and runs on any platform, so not sure what you're getting at here.

aethelwulffe commented 7 years ago

Hi Ken, Thank you for the detailed answer. I really am trying to learn here, and I do have the assumption that you know what you are talking about. Installing composer involves a binary file, and a path registration...not sure how that makes it just a php script. If, once the files are in the deployed application (something folks drop into www or htdocs directory) the composer files themselves can, and do, with no other external steps do something to support the upgrades and installation in sync with being able to just replace their existing files from the repo, or some future upgrade utility then I naturally support this.

The "Messy" comment: When folks pop up a nifty repo somewhere for some asset, usually that asset has assets, source files, documentation, migrations, more internal composer files...you name it. I could be wrong, and the .json file allows specificity to replace "jquery.custom.webasaurus.1.7.2.3.min.js" with "jquery.custom.webasaurus.1.7.2.4.min.js" cleanly and will never just include any additional directories et cetera. What I actually see is that when we started this fork, about 60% of the files in the project were useless bloat. This may not have had much (it did some) with the use of composer, but that has made me suspicious of anything not entirely self-contained. Hunting down components and identifying things has, despite having some information in a .json file (that was only meaningful if you already knew what you were looking at) been quite a trial. Thus, the FileNazi attitude on my part. Please forgive me for that.

So, Ken, obviously we WANT the power of composer. We can't ever ASSUME that everyone is going to, or be able to use it. With that in mind, I support any scheme that allows us to have the central composer directory system (with a nice readme in the directory that stays at the top, like 1_readme.md) that says "Composer dudes! all yer stuff iz in here! My impression was that it is difficult to use composer with any directory structure that does not specifically match its needs. I would rather we separated a "plugin" system like a "module" system at a top-level directory. Hierarchy seems to be the internal restriction here.

kchapple commented 7 years ago

If this is a permanent feature, I'd suggest moving it out of the Tags plugin and into it's own library file so it won't break anything if the Tags/Filter plugin is removed.

Jdew192837 commented 6 years ago

Is this something I could do @tmccormi @aethelwulffe @teryhill ? Or does this require further discussion?

tmccormi commented 6 years ago

@Jdew192837 Moving it out of the Tags plugin and into it's own library file so it won't break anything if the Tags/Filter plugin is removed.