#Contributing
balmy jolt
I'll be honest, it's been a while since i had to regen the generated directory....
Fix a bad detection of windows / darwin (looking for string win first creates a mismatch darwin for windows)
If you're talking about theCliDownloaderthat is@deprecatedand realistically won't be in use by anything as it was deprecated before the PHP SDK was promoted publicly. It wouldn't hurt to fix it, but ideally we want to remove them.
The issue with removing them is, put simply, because it was written by someone else... so I don't fully know how it all works.
It's going to take some time to dig in before I have confidence it can be removed.
bright remnant
no worries I can dig in... But I need the basics understanding of how the generated works. I'd be very happuy to help out more on the php sdk but the documentation to get started is very poor :/
balmy jolt
Use / rework Dagger attributes to ease things a bit
Happy to discuss any ideas, just be aware some of the decisions of which attributes to use is based on being consistent with other SDKs
bright remnant
dagger call sdk php generate export --path=./sdk/php/tmp seems to be doing teh trick but it is taking a huuuuuge hammer to smash a mosquito !
balmy jolt
From a quick play, looks like you can call:
<project-root>/sdk/php $ ./scripts/codegen.php
But awkwardly, it requires the generated directory existing first.
bright remnant
yes calling this is not working because : It looks for env variables set up by dagger, it use the CliDownloader instead (because of env variabels missing) and still requires the schema which I cannot generate because of what mentionned previously :/
balmy jolt
cc @fringe crater @urban light is there a proper way to regenerate the sdk code? Or does it vary per SDK?
Right I see.
bright remnant
something easier and quicker than generating the full sdk with dagger call sdk php generate export --path=./sdk/php/tmp
small one but should be fixing the deprecations we talked about. https://github.com/dagger/dagger/pull/11131 not sure if something else is required for me to do to properly contribute this...
it causes BC break if devs aren't using the named arguments feature of PHP. but it is always required to do this change.
balmy jolt
If it passes CI, my thoughts are to merge it for the 0.19 release, which will break BC anyway
bright remnant
let me know if I need to change / add /remove something from the PR.
balmy jolt
We'd want to add a release note.
Requires changie installed and then you call dagger/sdk/php $ changie new and it walks you through the process.
And if you can change the target branch of the PR to releases/0.19.0 so that we can tie it all in
I'd like to sit down and look into the introspection part when I review it, so I don't have time at the moment... but thank you for contributing 🙂
I sat down, it looks good to me!
balmy jolt
I can add a changelog if you prefer
bright remnant
I'll take a look at those tools. thank you for your help
balmy jolt
I'll take a look at those tools. thank you for your help
I pushed to your PR to add the changelog, I noticed nothing else is holding back a 0.19.0 release so I'm hoping we can get it in, in time. 🚀
balmy jolt
@fringe crater if we can, we'd like to get this one into 0.19.0 as well https://github.com/dagger/dagger/pull/11131
It's having some slight issue but I'm not certain what they mean https://dagger.cloud/dagger/traces/ddda314aee047e8f4aebafe5f79c3d3e
It just says changes detected
fringe crater