ext-php-rs
ext-php-rs copied to clipboard
davidcole1340
fix: linter warnings
When compiling the linter complained that php8x was not defined. I fixed it using rustc-check-cfg as defined here.
The compiler also complained that cfg(docs) was invalid. It's because it should be cfg(doc). I updated that. After updating, the compiler mentioned that feature(doc_cfg) was only available in nightly so I update the embed Dockerfile to reflect that.
Hey, thank you for your patience, I'm not the lead maintainer for now and as I don't work with PHP, I try my best to answer question 😅 the CI seems broken here but anyway, for the Dockerfile, is there a way to rely on nightly only for the doc problem? Because, it executes the test as well with nightly. cc @danog
@Xenira yeah, but considering @danog didn't seem to approve your changes, I went for a smaller change so that we can iterate.
@jdrouet ye, just wanted to make sure you've seen it.
tbh i don't know what to change to get it merged. Did the bare minimum to get ci woking again.
Agreed. I don't know if this repository will move forward at anytime soon. Maybe @danog or @davidcole1340 would consider moving this repository to an org with more people that could contribute?
If it doesn't Ill probably create my own repo. Have a bunch of changes locally that I would like to contribute, but am blocked by CI and can't wait 2 months on each of those.
No hard feelings towards the maintainers. Just want to move forward.
I'm happy to move this repo out into an organization - will do this later this week, but if you think there should be more approvers/maintainers please reach out to me! https://github.com/davidcole1340/ext-php-rs/issues/140
Regarding CI:
- Stable and unstable are tested in CI. These changes only work on unstable (https://github.com/davidcole1340/ext-php-rs/actions/runs/10935703203/job/30365113910?pr=324#step:12:256)
- Macos will also fail because of clang/llvm (#320)
I think on common issue could be the use of unstable features. Removing those would fix some CI issues.
@jdrouet I rebased your branch on master, please review that the logic is still correct
Without looking further into this and correct me if I am wrong:
Merging this would mean we loose stable support. Would like to avoid that. Guess same point as https://github.com/davidcole1340/ext-php-rs/pull/324#discussion_r1766474253 made.
@Xenira the problem is that this crate has many dependencies on nightly features. This introduces some errors at build time and makes it harded to reproduce the build. It can also be unstable over time considering the nightly features are, by definition, not sure to remain the same or land in stable. I'd suggest to remove them to only rely on stable features. I can do it if you agree with that.
@jdrouet maybe I don't understand the problem, but what exactly is not working with stable right now.
master is building just fine using the stable channel.