This issue is for catching the changes that have happened since the start of the conversion for Drupal 8.

There are at least 2 issues:

* The initial conversion was made using DrupalModuleUpgrader module. Since the initial port that module has had some updates and catches a few more changes that need to be made.
* 7.x had additional features after the initial conversion. For instance the ability to put a wrapper around icons.

The plan is to generate a patch for 8.x-dev which captures these changes.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Jeff Veit created an issue. See original summary.

Jeff Veit’s picture

FileSize
40.72 KB

1st step/patch. It's here to show some history and working, but it's not interesting in itself. It'll be incorporated in a later patch.
It takes the 7.x version of the module used as the basis of the 8.x module, runs it through DrupalModuleUpgrader as it now stands, and merges with all the changes in the 8.x-dev module at e9b19b38 - i.e. after the original DrupalModuleUpgrader update to 8.x. So it represents how 8.x would have been initially if the current DrupalModuleUpgrader module had been used.

Jeff Veit’s picture

FileSize
42.36 KB

More working. This is 7.x-1.x upgraded using DrupalModuleUpgrader.

Jeff Veit’s picture

FileSize
56.36 KB

This patch contains all the changes of the previous two, against 8.x-dev, resolving the issues originally listed.
It still has lots of FIXME's generated by the DrupalModuleUpgrader auto upgrade but it's contains the functionality of 7.x-1.0 + 8.x-dev changes + enhanced auto upgrade changes.

It will have to be re-rolled since there's been a new 8.x-1.x version while I was working on it.

Jeff Veit’s picture

It won't have to be rerolled yet. The change I saw was a patch, not a repo change.

Jeff Veit’s picture

Same patch, now with a more sensible file name.

markhalliwell’s picture

Status: Active » Needs work

This is reverting the icon_field work that was already committed from #2529032-23: Upgrade To Drupal 8.

Jeff Veit’s picture

Thanks, I'll have a look.

Jeff Veit’s picture

Thanks, I'll have a look.

Jeff Veit’s picture

Yup. That last one was still a work in progress, as is this one, but it merges in the the changes already committed to the 8.x-1.x branch - i.e the work on fields.

In this version there are still the numerous FIXMEs introduced by the auto upgrade. Dealing with those is the next job.

sylus’s picture

sylus’s picture

Looks like I am going to need this in order to continue my work on annotations.

#2863343: Create plugin managers for existing procedural D7 API

Would this also be the right place to take into account:

#2135291: [Policy, no patch] PHP 5.4 short array syntax coding standards for Drupal 8 ?

Jeff Veit’s picture

We could combine now. It's not working yet but if we put it all on github, then we can use git to bring it all up to date.

Or if you are using git - I can give you the repo I'm using. That would give us the same history and would mean it's easier to merge separate strands later on.

I haven't been doing coding standard fixes or prettyfying, but I could. However I'm probs not going to be able to work on it this weekend: seriously ill relative.

sylus’s picture

No worries appreciate your help!

I think @markcarver wanted us to stick to the patch workflow though.

I'll probably take your patch currently posted here and just complete what is left as I think there are only a few @FixMe's to do. Will of course leave attribution to you :)

Jeff Veit’s picture

I'm still saying to make patches so that Mark and others can track. It's just that it's easier if you have the repo.

I've got a few more changes made after the last patch - I was starting on FIXME items.

Patch in a minute or few...

Jeff Veit’s picture

Small changes but related to the discovery.

Patch against the current 8.x-1.x.

markhalliwell’s picture

FWIW, I'm ok with the current @FIXEME comments. The primarily goal of this issue (in my mind) is to merge in the wrapper changes from 7.x. Since this does that, I'd say we can go ahead and commit.

@Jeff Veit, once you're done, please un-assign and set the status to needs review.

@sylus, please review and merge once you're satisfied with the result.

Jeff Veit’s picture

I can carry on with FIXMEs in another thread. The main merge is done. Unassigning and setting to needs review now.

Jeff Veit’s picture

Assigned: Jeff Veit » Unassigned
Status: Needs work » Needs review

  • sylus committed 1c2ee7a on 8.x-1.x authored by Jeff Veit
    Issue #2863487 by Jeff Veit: Incorporating 7.x updates into 8.x-dev
    
sylus’s picture

Status: Needs review » Fixed

I tested this on my local with no problems. ^_^

Only noticed a few minor nits + whitespace so made the adjustments.

Committed and attributed, thank you so much for taking the time!

  1. +++ b/icon.links.menu.yml
    @@ -3,3 +3,12 @@ icon.admin:
    +icon.admin.bundles
    +  title: Bundles
    +  description: 'Overview of all available icon sets.'
    

    Minor correction to yml to include ':'

  2. +++ b/icon.permissions.yml
    @@ -1,4 +1,10 @@
    +  description: 'Grants selected roles full administrative permissions for all aspects of the Icon API. It supersedes all other icon permissionss.'
    +  restrict access: true
    

    Minor spelling nit.

Status: Fixed » Closed (fixed)

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