Skip to content

Conversation

@andybroomfield
Copy link
Contributor

Fix #232

Set the minimum version to 1.27 which addresses PHP 8.2 errors.

What does this change?

Updates office hours version to latest.

How to test

Localgov contact paragraph doesn't break on PHP 8.2

How can we measure success?

It works

Have we considered potential risks?

It doesn't work.

Images

n/a

Accessibility

n/a

Fix #232

Set the minimum version to 1.27 which addresses PHP 8.2 errors.
@andybroomfield
Copy link
Contributor Author

andybroomfield commented Apr 24, 2025

Failing test is PHP 8.4

@ekes
Copy link
Member

ekes commented Apr 29, 2025

#241

@finnlewis
Copy link
Member

From Merge Tuesday: agreed to merge #241 for now and continue the discussion here on pinning vs more loose version constraints.

CC @millnut

@andybroomfield
Copy link
Contributor Author

I think we treat this like how we lock the version for search_api_location module at the moment due to the recent instability and multiple breaking releases due to the maintainer adding PHP 8.3+ functionality and breaking 8.1 and 8.2

The concern here is that is we force a specific version of a contrib module which has security support, it is going to be difficult for sites to upgrade to security supported versions without using aliases in composer. There might also be other needs to pin to specific versions of office hours that differ from here which will now result in having to pin to previous versions of localgov_paragraphs.

Would adding a conflict section to this modules composer.json top indicate the versions that don't work resolve the issue.

If we can't trust the module, do we want to start looking at a replacement or fork?

@andybroomfield
Copy link
Contributor Author

Ref is this the underlying issue https://www.drupal.org/project/office_hours/issues/3514975

@millnut
Copy link
Member

millnut commented Apr 29, 2025

@andybroomfield I think the main problem is that the maintainer keeps adding support for PHP functionality that only exists in PHP 8.4 and only testing in PHP 8.4, which keeps causing repeated issues with users on PHP 8.1, 8.2 and 8.3.

I keep meaning to create a PR upstream to add gitlab ci for office_hours so that it will test against older versions of PHP and not just latest.

@AWearring
Copy link
Contributor

AWearring commented Jun 2, 2025

Is there any possibility of getting this unpinned - v1.27 is not covered by the Drupal security advisory policy

@millnut
Copy link
Member

millnut commented Jun 3, 2025

Is there any possibility of getting this unpinned - v1.2.7 is not covered by the Drupal security advisory policy

@AWearring we could discuss unpinning however the point still stands around this comment #243 (comment) where the maintainer could introduce future functionality which breaks PHP 8.1 through to 8.3 again.

It's rare that a contrib module is locked to a specific version, the only other case I can think of is search_api_location which is locked due to stability reasons as well.

@AWearring
Copy link
Contributor

AWearring commented Jun 3, 2025

Is there any possibility of getting this unpinned - v1.27 is not covered by the Drupal security advisory policy

@AWearring we could discuss unpinning however the point still stands around this comment #243 (comment) where the maintainer could introduce future functionality which breaks PHP 8.1 through to 8.3 again.

It's rare that a contrib module is locked to a specific version, the only other case I can think of is search_api_location which is locked due to stability reasons as well.

Hi @millnut How about pinning it to the latest version for now (v1.28) which will temporarily stop the warnings in Drupal admin? I can raise a PR to do this, if this is agreed - PR created in readiness (#248)

@millnut
Copy link
Member

millnut commented Jun 3, 2025

Hi @AWearring yeah I think that is the best approach to this one

@finnlewis finnlewis marked this pull request as draft June 10, 2025 11:29
@finnlewis
Copy link
Member

Converting this to draft for now.

We'll bring it back later.

Meanwhile, we can discuss php support here: https://www.drupal.org/project/office_hours/issues/3529359

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Office Hours 1.24 causes fatal errors

6 participants