Skip to content

[CI] Check that headers are up to date in the CI pipeline #35

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

Merged
merged 1 commit into from
Feb 24, 2023

Conversation

phansys
Copy link
Contributor

@phansys phansys commented Feb 22, 2023

@phansys phansys force-pushed the stub_ci branch 5 times, most recently from ee213d9 to 06f075d Compare February 22, 2023 22:17
@phansys phansys mentioned this pull request Feb 22, 2023
@Girgias
Copy link
Member

Girgias commented Feb 23, 2023

This PR looks completely nonsensical, you are using a documentation flag for something that doesn't seem to do any comparison to docs.

If what you are after is checking that the arginfos are correctly generated than copy part of https://github.com/php/php-src/blob/master/.github/actions/verify-generated-files/action.yml

@phansys
Copy link
Contributor Author

phansys commented Feb 23, 2023

This PR looks completely nonsensical, you are using a documentation flag for something that doesn't seem to do any comparison to docs.

The intention of this PR is checking if the contents in ibm_db2.stub.php produces any change in ibm_db2_arginfo.h or ibm_db2_legacy_arginfo.h.
The resulting XML documenaton is not under the SCM in this repo, so there is no check against those files here.

Please, let me know if I am misunderstanding the purpose of ibm_db2_arginfo.h and ibm_db2_legacy_arginfo.h. To me, given the way they are currently generated by gen_stub.php, they seem an artifact that must be in sync with its source (ibm_db2.stub.php).

@phansys
Copy link
Contributor Author

phansys commented Feb 23, 2023

@Girgias, sorry for the ping.

I've removed the --replace-methodsynopses option from the gen_stub.php execution. Could you please confirm if it makes sense for you?
Thank you in advance.

@phansys phansys marked this pull request as ready for review February 23, 2023 16:51
@Girgias
Copy link
Member

Girgias commented Feb 24, 2023

Looks fine to me now.

@NattyNarwhal NattyNarwhal merged commit 74eea5f into php:master Feb 24, 2023
@phansys phansys deleted the stub_ci branch February 24, 2023 15:33
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