Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refresh Code base #21

Merged
merged 7 commits into from
Oct 12, 2020
Merged

Refresh Code base #21

merged 7 commits into from
Oct 12, 2020

Conversation

rieschl
Copy link
Contributor

@rieschl rieschl commented Oct 11, 2020

  • remove custom autoloaders
  • migrate to Laminas
  • fix test namespace
  • (mostly) apply Laminas Coding Standard
  • add GitHub Action
  • upgrade to laminas-mvc 3 and laminas-eventmanager 3

Sorry this is a very very big PR. Normally I don't do that because reviewing is quite hard that way, but the code base is very old and probably has a low usage, it's much easier than separate PRs.
But the commits are somewhat divided: the first one is the big one with Laminas, code style and code quality changes and the upgrade from mvc and eventmanager v2 to v3 is in a separate commit. The commits after that are bug fixes and forgotten stuff.

Edit: ... and I tested it on a freshly installed laminas-mvc skeleton 🙂

fixes #15

@rieschl
Copy link
Contributor Author

rieschl commented Oct 11, 2020

Hmm, are GitHub Actions disabled in this repository? The pull_request event should trigger the new action here. The push action worked in my fork https://github.com/rieschl/PhlyBlog/actions

composer.json Outdated
"laminas/laminas-uri": "^2.5",
"laminas/laminas-validator": "^2.5",
"laminas/laminas-view": "^2.5",
"phly/phly-common": "dev-refresh"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This line and the repositories block will be removed after PhlyCommon was merged and tagged.

"require": {
"phly/phly-blog": "dev-master"
"phly/phly-blog": "^3.0"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

It'll be tagged as 3.0.0, right?

Copy link
Contributor

Choose a reason for hiding this comment

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

I never did a 2.0, so this would be 2.0 here. :)

@vrkansagara
Copy link
Contributor

@rieschl and @weierophinney this looks good to me. 💯

rieschl and others added 7 commits October 12, 2020 10:51
* migrate to Laminas
* fix test namespace
* (mostly) apply Laminas Coding Standard
* add GitHub Action
* use iteratorToArray from StdLib
* correctly detach event listeners
* fix call to ServiceManager in Controller
@weierophinney
Copy link
Contributor

@rieschl Actions have to be present in the default branch in order to run, IIRC.

@weierophinney weierophinney added this to the 2.0.0 milestone Oct 12, 2020
@weierophinney weierophinney changed the base branch from master to develop October 12, 2020 16:03
@weierophinney weierophinney merged commit ca4d1cc into phly:develop Oct 12, 2020
@weierophinney weierophinney mentioned this pull request Oct 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Any plan to migrate this package to Laminas ?
3 participants