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

Integrates Autoloader and FileLocator #1562

Merged
merged 27 commits into from
Dec 8, 2018

Conversation

natanfelles
Copy link
Contributor

Description

Fixes #1552

Checklist:

  • Securely signed commits
  • Component(s) with PHPdocs
  • Unit testing, with >80% coverage
  • User guide updated
  • Conforms to style guide

@natanfelles natanfelles changed the title WIP - Integrates Autoloader and FileLocator Integrates Autoloader and FileLocator Dec 2, 2018
@jim-parry
Copy link
Contributor

It looks like this PR got away on you a bit... it includes

  • CLI & CommandRUnner localization
  • Common refactoring
  • Language enhancements (getLocale)
  • Logger refatoring
  • Services testing (email, newLanguage)
  • Validation test changes

Is this all part of autoload/filelocator enhancement? There's a lot more here than is mentioned in 1552!

@natanfelles
Copy link
Contributor Author

No. The Logger is not related. But PhpStorm highlighted the constructor type hint error when I was in the Services, then I did.

@jim-parry
Copy link
Contributor

This looks good to go.
@natanfelles is that correct?
@lonnieezell are you ok with this?

@jim-parry jim-parry added this to the 4.0.0-alpha.4 milestone Dec 7, 2018
Copy link
Member

@lonnieezell lonnieezell left a comment

Choose a reason for hiding this comment

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

Yep - no problems with this, just a few questions to understand.

system/Autoloader/FileLocator.php Show resolved Hide resolved
system/Common.php Show resolved Hide resolved
system/Config/Services.php Show resolved Hide resolved
}

return new \CodeIgniter\Autoloader\FileLocator(new \Config\Autoload());
Copy link
Contributor Author

Choose a reason for hiding this comment

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

To solve issue 1552, it all started in this line. Using a new \Config\Autoload() it was not possible to dynamically use Autoloader namespaces.

@natanfelles
Copy link
Contributor Author

@jim-parry Yes. I believe that is correct.

@jim-parry jim-parry merged commit 29ca3bc into codeigniter4:develop Dec 8, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants