search

andig / videodb

The videoDB media collection software
65 stars 42 forks source link

Consolidate configuration options #161

Closed johanneskonst closed 1 year ago

johanneskonst commented 1 year ago

moved all pdf and xls configuration into the default config.*.php files. am now using the sample file as blueprint for available settings had to fiddle with the constants to get no BC breaks. only drawback is that when people've used the pdf.inc and/or xls.inc files those settings are not automatically used. but that's something we can fix in the upgrade.php script

johanneskonst commented 1 year ago

@copperhead57 any more info on why you get the 500? only reason i can think of this quickly is the define() statements in config.inc but i thought i mitigated that...

  1. agree, required code is in repo already (or will be in release) so enabling those by default would do no harm
  2. agreed, all config options need more documentation, will amend soon
  3. 👍🏼
copperhead57 commented 1 year ago

@johanneskonst i am stumped why 500. nothing is showing in php error log. maybe that the 2 included statements for xls and php are not found.

johanneskonst commented 1 year ago

Ah, yes, doing a require_once on a non-existent file might fail hard indeed. But people already having a config.inc are going to do an upgrade and probably still have pdf.inc and xls.inc in place so hopefully no error there. I'll put that in install/upgrade.php lateron.

copperhead57 commented 1 year ago

definitely - have reproduced the 500 by including one of the require_once on a non-existent file

copperhead57 commented 1 year ago

I am still getting the 500 on the old config.inc is not being modified with new options and deletion of deleted files

johanneskonst commented 1 year ago

Are you getting the 500 because of the missing files (pdf.inc / xls.inc) or is it something else? I havent done any work on the update process yet, you want me to focus on that first?Message ID: @.***>

copperhead57 commented 1 year ago

yes because of pdf,inc and xls.inc missing. i have updated my config.inc in-line with sample and all is fine my only concern is for people that upgrade to new master without realising that they need to change their config.inc and get 500 error and issues maybe raised.