search

netresearch / t3x-rte_ckeditor_image

Image support in CKEditor for the TYPO3 ecosystem
GNU Affero General Public License v3.0
56 stars 65 forks source link

Fix build errors - make functional test work with v13 #272

Closed sypets closed 9 months ago

sypets commented 9 months ago

This is not meant to be merged, it is a demo only.

Branch fix_build_errors was used, PR #271

Unfortunately, the functional test now shows a deprecation warning, but it will return 0 (ok).

1 test triggered 1 deprecation:

1) /home/peters/prog.local/github-pull-requests/typo3-extensions/rte_ckeditor_image/.Build/vendor/typo3/cms-core/Classes/Utility/ExtensionManagementUtility.php:614
ExtensionManagementUtility::addPageTSConfig() has been deprecated in TYPO3 v13.0 and will be removed in v14.0. Use Configuration/page.tsconfig files in extensions instead.

Triggered by:

* Netresearch\RteCKEditorImage\Tests\Functional\DataHandling\RteImageSoftReferenceParserTest::updateReferenceIndexAddsIndexEntryForImage (2 times)
  /home/peters/prog.local/github-pull-requests/typo3-extensions/rte_ckeditor_image/Tests/Functional/DataHandling/RteImageSoftReferenceParserTest.php:21

We could get rid of that by changing FunctionalTests.xml to be less strict with deprecations.

(I find it difficult to support 2 TYPO3 versions in one branch of an extension. I personally like to be strict and not have any deprecations at all, so I don't do it. But I realize, some (a lot of) extension authors like to support 2 TYPO3 versions in one branch.)

CybotTM commented 9 months ago

Hi @sypets,

thank you very much, will look into it when back from vacation.

regarding the version in compose.yml I also think that this is unnecessary and even outdated, I dropped it on purpose:

The top-level version property is defined by the Compose Specification for backward compatibility. It is only informative.

https://github.com/compose-spec/compose-spec/blob/master/04-version-and-name.md

Why do you think it is required?

sypets commented 9 months ago

regarding the version in compose.yml I also think that this is unnecessary and even outdated, I dropped it on purpose:

When running the script locally, I got error messages. Using the version, it disappeared. To be honest, I am not terribly familiar with the details.

Long term it might make more sense to replace the docker-compose part, core and "tea" extension are going with a docker / podman solution without docker-compose and runTests.sh script has changed considerably:

The error I got without version was:

 # Build/Scripts/runTests.sh -s functional

ERROR: The Compose file './docker-compose.yml' is invalid because:
Unsupported config option for services: 'mariadb10'
ERROR: The Compose file './docker-compose.yml' is invalid because:
Unsupported config option for services: 'unit'
DavidBruchmann commented 9 months ago

Thanks for the info with the version, never had a case like that yet.

CybotTM commented 9 months ago

Hi @sypets

Unsupported config option for services: 'mariadb10'

May this be caused by an outdated compose CLI version? Remember similar behavior only with compose CLI v1

See https://docs.docker.com/compose/compose-file/compose-versioning/#versioning

Version 1. This is specified by omitting a version key at the root of the YAML.

compose CLI v1 would fall back to compose file definition v1 if version is omitted. Which would explain the error, there is no "service" section in compose file definition v1.

We can leave the version: 3, but i strongly suggest that you update your compose CLI to v2.

I assume compose CLI v1 also does not support compose.yml file name - which I tend to change too.

Just noticed you longer explanation - which is basically the same i wrote. Just the docker-compose doesn't need to be v1, v2 can also provide a docker-compose alias. There was a package for this.

CybotTM commented 9 months ago

Thanks all - will include relevant parts into https://github.com/netresearch/t3x-rte_ckeditor_image/pull/271