adixon / ca.civicrm.logviewer

CiviCRM Log Viewer
Other
9 stars 11 forks source link

pear log 1.14 compatibility #27

Closed demeritcowboy closed 1 month ago

demeritcowboy commented 2 months ago

In civi 5.76, it switches to pear/log 1.14 which no longer has any way to get the filename. But in 5.76 core now provides its own public function to do it.

Since the version of pear/log is fixed exactly in core, there is a hard cut between 5.75 and 5.76, regardless of cms, so I've proposed upping the minimum install version to 5.76, which sounds weird at first but since the new function isn't available until 5.76, it would fail on <5.76.

An alternative is support both if it was desired to have any new upcoming features available to <5.76.

adixon commented 2 months ago

Thanks! I think this is why they invented semantic versioning?

demeritcowboy commented 2 months ago

I suppose one advantage of supporting both is it would mean people could update logviewer ahead of time. It's not much extra to support both - see the cv PR: https://github.com/civicrm/cv/pull/199/files

adixon commented 1 month ago

Yeah, let's do the more sophisticated version like cv, since there are likely lots of people who will not upgrade to 5.76 for a while, and as per your example with cv, it doesn't look hard.

adixon commented 1 month ago

I've converted your pr to a new one https://github.com/adixon/ca.civicrm.logviewer/pull/30 and added a commit.