Skip to content

fix: change getFile() function of \CodeIgniter\Events\Events to static. - #7046

Merged
kenjis merged 4 commits into
codeigniter4:developfrom
ping-yee:refactor-static-getFile
Jan 6, 2023
Merged

fix: change getFile() function of \CodeIgniter\Events\Events to static.#7046
kenjis merged 4 commits into
codeigniter4:developfrom
ping-yee:refactor-static-getFile

Conversation

@ping-yee

@ping-yee ping-yee commented Jan 3, 2023

Copy link
Copy Markdown
Contributor

Description
Fixes #7045

But I have a question is does the $this->manager still be needed in the test case of test\system\Events\EventsTest.php file?

Because all the functions in Event class are static, does it mean that this class no longer needs to be constructed?

Checklist:

  • Securely signed commits
  • Component(s) with PHPDoc blocks, only if necessary or adds value
  • Unit testing, with >80% coverage
  • User guide updated
  • Conforms to style guide

@kenjis kenjis added the bug Verified issues on the current code behavior or pull requests that will fix them label Jan 3, 2023
@kenjis

kenjis commented Jan 3, 2023

Copy link
Copy Markdown
Member

$this->manager is no problem.
Because we can instantiate static classes with no problem.

By the way, can you remove $this->manager in the tests?

@ping-yee

ping-yee commented Jan 3, 2023

Copy link
Copy Markdown
Contributor Author

Ok, I'll handle this.

Comment thread tests/system/Events/EventsTest.php Outdated

@kenjis kenjis left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

See the inline comment.

@kenjis kenjis changed the title refactor: change getFile() function of \CodeIgniter\Events\Events to static. fix: change getFile() function of \CodeIgniter\Events\Events to static. Jan 6, 2023
@kenjis
kenjis merged commit d8d5275 into codeigniter4:develop Jan 6, 2023
@ping-yee
ping-yee deleted the refactor-static-getFile branch January 6, 2023 06:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Verified issues on the current code behavior or pull requests that will fix them

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug: The getFiles method of \CodeIgniter\Events class is declared as 'public', but it should probably be 'public static'.

3 participants