search

andig / videodb

The videoDB media collection software
65 stars 42 forks source link

Make xls spreadsheet included via composer #137

Closed copperhead57 closed 1 year ago

copperhead57 commented 2 years ago

xls.php - include vendor/autoload.php xls.php - left align plot xls.php - add test if imdbID field is blank to stop corrupt xls file amend config.sample.php

johanneskonst commented 1 year ago

As per #153, i'm all for adding external libraries to composer but not too happy with including all their source in videodb's git repo. if you don't mind i'd like to workout #153 first before (partially) merging this.

johanneskonst commented 1 year ago

@copperhead57 could you remove all changes in the vendor folder from this pull request? so that i can merge this with only the changes from composer.json, config.sample, core/xls and xls.inc (and after that merge all remaining PRs)

And what are your thoughts on merging all configuration options from xls.inc.php (and pdf.inc.php also) into the config.sample? I think that any 'performanceboost' from putting unused options in a separate file aren't even measurable these days, and it makes it predictable on where to find configuration options that might need changes...

copperhead57 commented 1 year ago

@johanneskonst I am not exactly sure how to do vendor folder changes manually as I am only novice user of composer (my IDE tool does all the work). As all the other PR's are independent of composer lib changes can they go ahead anyway?

As for merging config into config.sample a good idea but I have found out that it causes a problem if the user doesn't update their config.inc file. Without some method to auto update their file i foresee issues being raised. this is not a show stopper just an observation, I have done a quick change to this piece of work with a work around.

johanneskonst commented 1 year ago

Ah, i'll cleanup vendor later then, i just want to clear the list of PR and issues so we can move forward. And handling the missing config options is no problem, i'll cover that lateron...