Skip to content

Conversation

pbisaga
Copy link
Contributor

@pbisaga pbisaga commented Oct 5, 2021

Question Answer
JIRA issue https://issues.ibexa.co/browse/IBX-828
Type Improvement
Target version starts since 0.8
BC breaks no
Doc needed no

TODO:

  • Implement feature / fix a bug.
  • Implement tests + specs and passing ($ composer test)
  • Fix new code according to Coding Standards ($ composer fix-cs).
  • Ask for Code Review.

@pbisaga pbisaga requested review from mnocon and Steveb-p October 5, 2021 09:45
@pbisaga pbisaga changed the base branch from master to 0.8 October 5, 2021 10:02
@pbisaga pbisaga changed the base branch from 0.8 to master October 5, 2021 11:35
@pbisaga
Copy link
Contributor Author

pbisaga commented Oct 5, 2021

Previously closed by mistake.

@pbisaga pbisaga closed this Oct 5, 2021
@mnocon mnocon reopened this Oct 5, 2021
@mnocon mnocon changed the base branch from master to 1.0 October 5, 2021 14:22
@mnocon mnocon changed the base branch from 1.0 to 0.8 October 5, 2021 14:22
@mnocon mnocon changed the base branch from 0.8 to 1.0 October 5, 2021 14:22
@pbisaga pbisaga force-pushed the IBX-828-Moved-jobs-to-GitHubActions branch from c4e68d0 to 20105ea Compare October 5, 2021 14:27
@pbisaga pbisaga self-assigned this Oct 7, 2021
@mnocon
Copy link
Member

mnocon commented Oct 11, 2021

Same as in ezsystems/BehatBundle#217 (comment), we need to find out why the Actions are not running

Copy link
Member

@mnocon mnocon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The PHP 7.4 job fails with:

  Problem 1
    - Root composer.json requires phpspec/phpspec ^5.1.2 -> satisfiable by phpspec/phpspec[5.1.2].
    - phpspec/phpspec 5.1.2 requires php ^7.1, <7.4 -> your php version (7.4.24) does not satisfy that requirement.
  Problem 2
    - phpspec/phpspec 5.1.2 requires php ^7.1, <7.4 -> your php version (7.4.24) does not satisfy that requirement.
    - memio/spec-gen 0.9.0 requires phpspec/phpspec ^5.0@rc -> satisfiable by phpspec/phpspec[5.1.2].
    - Root composer.json requires memio/spec-gen ^0.9.0 -> satisfiable by memio/spec-gen[0.9.0].

Can we try to update PHPSpec so that it's possible to install it on PHP 7.4?

You'd need to:

  1. Change current requirement (https://github.com/ezsystems/ezplatform-http-cache/blob/1.0/composer.json#L21) to "^5.1.2 || ^6.1"
  2. Change the "memio/spec-gen": "^0.9.0", requirement to "^0.9.0 || ^0.10.0"

If it's not possible we will need to drop tests on PHP 7.4

@Steveb-p
Copy link

Steveb-p commented Oct 19, 2021

memio/spec-gen is this a direct dependency for us? Maybe it can be removed, since from the look of it the library seems to be abandoned, with last commits from 2 years ago.

EDIT: It's not even present on master branch anymore.

@mnocon
Copy link
Member

mnocon commented Oct 19, 2021

Yes, we can try removing it as well - then we need to also remove it from the phpspec config: https://github.com/ezsystems/ezplatform-http-cache/blob/1.0/phpspec.yml#L1-L2

Copy link
Member

@mnocon mnocon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, just small nitpick

phpspec.yml Outdated
Comment on lines 1 to 2

suites:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
suites:
suites:

No need for empty line at the beginning of the file

@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@micszo micszo merged commit 901a75d into 1.0 Oct 22, 2021
@micszo micszo deleted the IBX-828-Moved-jobs-to-GitHubActions branch October 22, 2021 07:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

4 participants