To make module compatible with 8.3 core it need to remove rest dependency

Details in CR https://www.drupal.org/node/2830467

CommentFileSizeAuthor
#27 2867574-file_entity-83-27.patch0 bytesmusa.thomas
#17 interdiff-13-17.txt494 bytesgnuget
#17 2867574-file_entity-83-17.patch2.63 KBgnuget
#13 2867574-file_entity-83-13.patch2.35 KBandypost
#13 interdiff.txt375 bytesandypost
#11 2867574-file_entity-83-11.patch1.99 KBandypost
#11 interdiff.txt1.53 KBandypost
#3 file_entity-83-3.patch12.83 KBwengerk
#2 file_entity-83-2.patch1.4 KBandypost

Comments

andypost created an issue. See original summary.

andypost’s picture

andypost
he/him
Primary language Russian
commented
Status: Active » Needs review
StatusFileSize
new file_entity-83-2.patch1.4 KB

This could be done in BC but I see no reason

wengerk’s picture

wengerk
he/him
Primary language French
Location Lausanne
commented
StatusFileSize
new file_entity-83-3.patch12.83 KB

@andypost seems your patch using the commit 724ec24 and not the tagged version 8.x-2.0-beta3 so people using composer to install the module and using the release cant apply your patch
I just create a patch for people from the 8.x-2.0-beta3 to the compatible version (21/04/2017 - commit 724ec24).

Patch from 8.x-2.0-beta3 to patched version

Status: Needs review » Needs work

The last submitted patch, 3: file_entity-83-3.patch, failed testing.

andypost’s picture

andypost
he/him
Primary language Russian
commented

Closed as duplicate #2877678: Update rest.link_manager dependency to use hal.link_manager service

andypost’s picture

andypost
he/him
Primary language Russian
commented

btw info file needs update as well to make module non-install-able for 8.2

andypost’s picture

andypost
he/him
Primary language Russian
commented
Status: Needs work » Needs review

@wengerk your patch is wrong, see issue summary link

l0ke’s picture

l0ke
Primary language English
Location Ukraine
commented
Status: Needs review » Reviewed & tested by the community 
+++ b/src/FileEntityServiceProvider.php
@@ -21,12 +21,11 @@ class FileEntityServiceProvider extends ServiceProviderBase {
-    // Check for installed REST and HAL modules. HAL does not require REST
-    // anymore in 8.3 and later.
-    if (isset($modules['hal']) && isset($modules['rest'])) {
+    // Check for installed HAL module.
+    if (isset($modules['hal'])) {

Nit: I don't think comment is necessary anymore. It had its sense before but now this check is quite obvious.

Mentioned nitpick is quite style-ish so RTBC'ing.

geekinpink’s picture

geekinpink commented

This patch works for me.

file_entity

williambe’s picture

williambe commented

works for me to.

andypost’s picture

andypost
he/him
Primary language Russian
commented
StatusFileSize
new interdiff.txt1.53 KB
new 2867574-file_entity-83-11.patch1.99 KB

reroll to fix nitpick, makes sense to create new release

joseph.olstad’s picture

joseph.olstad
Primary language French
commented

rolled some tests for 8.3.x
usind 8.4.x the testbot is complaining

we'll see what 8.3.x says

andypost’s picture

andypost
he/him
Primary language Russian
commented
StatusFileSize
new interdiff.txt375 bytes
new 2867574-file_entity-83-13.patch2.35 KB

Fix for tests

andypost’s picture

andypost
he/him
Primary language Russian
commented

Module tests are broken & it needs separate issue https://www.drupal.org/node/1212874/qa

joseph.olstad’s picture

joseph.olstad
Primary language French
commented

Fix head tests
#2892203: Fix tests for 8.3 core

gnuget’s picture

gnuget
Primary language Spanish
commented
Related issues: +#2878946: Fix in the FileEntityServicesTest::testFileFieldREST test

I wrote a patch which fix the tests on 8.3 a few months ago.

https://www.drupal.org/node/2878946

gnuget’s picture

gnuget
Primary language Spanish
commented
StatusFileSize
new 2867574-file_entity-83-17.patch2.63 KB
new interdiff-13-17.txt494 bytes

Here a patch which includes #2878946

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 17: 2867574-file_entity-83-17.patch, failed testing. View results

gnuget’s picture

gnuget
Primary language Spanish
commented
Status: Needs work » Reviewed & tested by the community

It seems that it didn't work :-( #13 is still the good one.

joseph.olstad’s picture

joseph.olstad
Primary language French
commented

requeued tests for #13

joseph.olstad’s picture

joseph.olstad
Primary language French
commented

I committed #2878946: Fix in the FileEntityServicesTest::testFileFieldREST test
but the head tests still fail
and #13 still fails as of today, I requeued it and still fail

joseph.olstad’s picture

joseph.olstad
Primary language French
commented

ok, followup on this test
https://www.drupal.org/pift-ci-job/731533

  • joseph.olstad committed 8d1f053 on 8.x-2.x authored by gnuget 
    Issue #2867574 by andypost, gnuget, wengerk: Core 8.3 compatibility
joseph.olstad’s picture

joseph.olstad
Primary language French
commented
Status: Reviewed & tested by the community » Fixed
andypost’s picture

andypost
he/him
Primary language Russian
commented

Yay! It's time to create new beta?

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

musa.thomas’s picture

musa.thomas
Primary language French
Location France 🇫🇷
commented
StatusFileSize
new 2867574-file_entity-83-27.patch0 bytes

here the patch #13 wich work with composer install