Skip to content

Add priority parameter to attach method in ManagerInterface#16816

Merged
niden merged 5 commits into
phalcon:5.0.xfrom
tashik:fix-managerinterface
Nov 10, 2025
Merged

Add priority parameter to attach method in ManagerInterface#16816
niden merged 5 commits into
phalcon:5.0.xfrom
tashik:fix-managerinterface

Conversation

@tashik

@tashik tashik commented Nov 8, 2025 •
edited by niden
Loading

Copy link
Copy Markdown
Contributor

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

niden commented Nov 8, 2025

Copy link
Copy Markdown
Member

@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

niden commented Nov 8, 2025

Copy link
Copy Markdown
Member

@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

tashik commented Nov 9, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

@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

niden commented Nov 9, 2025

Copy link
Copy Markdown
Member

@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

tashik commented Nov 10, 2025

Copy link
Copy Markdown
Contributor Author

@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
Code quality issue in Phalcon\Events\ManagerInterface.zep #16817
Closed
@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

niden commented Nov 10, 2025

Copy link
Copy Markdown
Member

Thank you @tashik

@niden niden mentioned this pull request Nov 10, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@niden niden niden approved these changes

Assignees

No one assigned

Labels

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

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Code quality issue in Phalcon\Events\ManagerInterface.zep

3 participants

@tashik @niden @SidRoberts