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.

Sign up for GitHub

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

Jump to bottom

Don't add the listener if a document is not a test file #8196

Merged
merged 1 commit into from
Jan 28, 2025
Merged

Don't add the listener if a document is not a test file #8196

merged 1 commit into from
Jan 28, 2025
+126 −69

Conversation

junichi11
Copy link
Member

@junichi11 junichi11 commented Jan 26, 2025 •
edited
Loading

  • Avoid adding the listener to all PHP files (Add it to only test files) because other than test files don't have test methods
  • Improve the deeply nested parts

@junichi11 junichi11 added the PHP [ci] enable extra PHP tests (php/php.editor) label Jan 26, 2025
@junichi11 junichi11 added this to the NB25 milestone Jan 26, 2025
@junichi11 junichi11 requested review from tmysik and troizet January 26, 2025 09:40
tmysik
tmysik reviewed Jan 26, 2025
View reviewed changes
Copy link
Member

@tmysik tmysik left a comment

Choose a reason for hiding this comment

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

@junichi11 , please, see my comments, thank you.

@junichi11 junichi11 force-pushed the php-improve-test-method-annotations branch from eb40f84 to b4fcdad Compare January 26, 2025 14:55
@junichi11
Don't add the listener if a document is not a test file
c6eb9a8 
- Avoid adding the listener to all PHP files (Add it to only test files)
  Because other than test files don't have test methods
- Improve the deeply nested parts
@junichi11 junichi11 force-pushed the php-improve-test-method-annotations branch from b4fcdad to c6eb9a8 Compare January 26, 2025 15:27
troizet
troizet approved these changes Jan 26, 2025
View reviewed changes
Copy link
Collaborator

@troizet troizet left a comment

Choose a reason for hiding this comment

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

Thank you very much for the improvements!

@junichi11
Copy link
Member Author

I made an elementary stupid mistake but I realized that thanks to Tomas.
A release manager merges this into the delivery branch :)
Thank you, both!

@junichi11 junichi11 mentioned this pull request Jan 27, 2025
Open
@junichi11
Copy link
Member Author

@ebarboni @lahodaj Conflict may occur because this file is changed here(delivery branch) and #8201 (master branch).

@ebarboni
Copy link
Contributor

will merge this and create sync but the #8201 will need rebase

@ebarboni ebarboni merged commit f2f8ea7 into apache:delivery Jan 28, 2025
32 checks passed
@junichi11 junichi11 deleted the php-improve-test-method-annotations branch January 28, 2025 23:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tmysik tmysik tmysik approved these changes

@troizet troizet troizet approved these changes

Assignees
No one assigned
Labels
PHP [ci] enable extra PHP tests (php/php.editor)
Projects
None yet
Milestone
NB25
Development

Successfully merging this pull request may close these issues.
4 participants
@junichi11 @ebarboni @tmysik @troizet