Skip to content

Conversation

@tashik
Copy link

@tashik tashik commented Nov 8, 2025

Hello!

In raising this pull request, I confirm the following:

  • I have read and understood the Contributing Guidelines
  • I have checked that another pull request for this purpose does not exist
  • I wrote some tests for this PR
  • I have updated the relevant CHANGELOG
  • I have created a PR for the documentation about this change

Small description of change:

This pull request fixes #16817

My mini-fix will allow to generate correct ide-stubs. Please, update ide-stubs accordingly if possible.

Thanks,
Natalya

@niden
Copy link
Member

niden commented Nov 8, 2025

@tashik Thank you for this.

Could you please create an issue first, then link it with this PR?
Also, we do not accept PRs towards the master branch so you need to rebase that to point to the v5.0.x

Thanks!

@niden
Copy link
Member

niden commented Nov 8, 2025

@tashik Thank you for this.

Could you please create an issue first, then link it with this PR? Also, we do not accept PRs towards the master branch so you need to rebase that to point to the v5.0.x

Thanks!

Also you might want to use the constant defined in the Manager (DEFAULT_PRIORITY). Move it to the Interface and use it from there

@tashik tashik force-pushed the fix-managerinterface branch from 18e9394 to a5004a7 Compare November 9, 2025 09:54
@tashik tashik changed the base branch from master to 5.0.x November 9, 2025 09:55
@tashik
Copy link
Author

tashik commented Nov 9, 2025

@niden, I have fixed my pull request, created an issue #16817 and connected the PR and the issue. I have rebased my branch to 5.0.x, but there are two commits there which are not mine.

@niden
Copy link
Member

niden commented Nov 9, 2025

@niden, I have fixed my pull request, created an issue #16817 and connected the PR and the issue. I have rebased my branch to 5.0.x, but there are two commits there which are not mine.

Thanks for that. Don't worry about the other commits, they probably came from your rebase.

One last thing if you can. For Zephir we do not have public const it is just const. The code does not compile with the public in there. (see CI run). If you can remove that, we can get the suite to run and then merge it if all is OK.

@tashik
Copy link
Author

tashik commented Nov 10, 2025

@niden I have removed constant visibility, sorry for that, I am not familiar with Zephir.

@niden niden linked an issue Nov 10, 2025 that may be closed by this pull request
@niden niden added bug A bug report status: low Low 5.0 The issues we want to solve in the 5.0 release labels Nov 10, 2025
@niden niden merged commit ece6960 into phalcon:5.0.x Nov 10, 2025
42 checks passed
@niden
Copy link
Member

niden commented Nov 10, 2025

Thank you @tashik

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

5.0 The issues we want to solve in the 5.0 release bug A bug report status: low Low

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Code quality issue in Phalcon\Events\ManagerInterface.zep

3 participants