search

silverstripe / silverstripe-globaltoolbar

Embeddable toolbar showing at the top of *.silverstripe.org community sites
BSD 3-Clause "New" or "Revised" License
4 stars 13 forks source link

SS4 Compatability #42

Closed unclecheese closed 7 years ago

unclecheese commented 7 years ago

First pass at making the toolbar ss4 compatible.

helpfulrobot commented 7 years ago

@unclecheese, thanks for your PR! By analyzing the blame information on this pull request, I identified @halkyon to be a potential reviewer

robbieaverill commented 7 years ago

It's also a bit of a shame to lose the Git history on those migrated classes. Any chance you wouldn't mind re-doing it so they're retained in separate commits?

robbieaverill commented 7 years ago

@unclecheese everything looks great! I've just PRd a couple of changes - one for an updated Versioned API requirement, a commit for PSR-2 linting and a commit for updating a couple of ClassName values in the templates, and adding $ for function calls. They're all in #43

I've tested this on a local copy of ss.org and haven't noticed any regressions, so 👍 from me once #43 is in.

Noted:

There are references in the templates to BlogHolder which is a deprecated compatibility class in the blog and is removed in the SS4 compatible version - do we need to update this to look directly at Blog instead? I'm not sure of the context here.