pine plover
@sand horizon I seem to have hit some nightmare where ./hack/make sdk:go:lint passes consistently for me locally and fails consistently in CI for a reason I don't understand: https://github.com/dagger/dagger/actions/runs/6473535043/job/17576448433?pr=5757#step:4:143
If you have a sec, do you mind pulling the latest zenith-functions branch and seeing what happens when you run the go sdk lint? Curious how much of a "me problem" it is
sand horizon
trying it out!
@pine plover works fine locally here too. I wonder if remote is somehow looking at the 'latest shipped' dagger.io/dagger or something...? since that import is new on the branch
actually, maybe this is where it compares to main? i have a vague memory of that
pine plover
wait... I think I found it...
gitignore was still ignoring internal/querybuilder
lol
sand horizon
pine plover
thank you for being my soundboard... 😅
pine plover
@sand horizon okay now running into something real I think. ./hack/make sdk:go:test fails for a few reasons, the biggest being that we are passing a nil dagger client here: https://github.com/shykes/dagger/blob/f315ee3abc42d9e24fb587e047551fb9a7011700/sdk/go/codegen/generator/nodejs/templates/functions_test.go#L17
which causes a panic when it tries to get the current schema w/ an introspection query.
The problem is that the fix is to create a dagger client, which requires adding a dependency to the go sdk on the engine, which is our prime directive not to do since it created so many nightmares in the past.
What's the reason all that got moved to be under sdk/go? I'm wondering/hoping I can just move it back to the engine module, but want to check first
May also be able to refactor those unit tests to just not rely on the "real" introspection result, looking more closely it's not clear to me that it's actually important they use the real live one
Still curious though
sand horizon
There's definitely some confusing things I'm happy to explain now that this all moved around, but I'm not seeing where we would need a dependency from the engine => Go SDK. Or do you mean Go SDK => engine?
All of the Go codegen moved under sdk/go/ as part of module-ifying it, since otherwise the SDK codegen depended on github.com/dagger/dagger which was a total nightmare
NodeJS codegen came along for the ride, which is hilariously confusing and needs to be fixed up
pine plover
There's definitely some confusing things I'm happy to explain now that this all ...
Yeah that's what I was trying to say, arrow notation is more clear 🙂
Okay cool, I'm just going to refactor them to use a canned introspection result so doesn't actually matter for now, but good to know, thanks!
sand horizon
Another thing worth mentioning is that the "Go SDK" now also includes code for things like loading module refs, which is used by both the CLI and by Daggerverse. I think this makes sense, since those are generally useful to anyone writing tooling for Dagger, so it makes sense to include in a Dagger SDK. (Maybe other SDKs would end up with equivalent implementations someday.)
Once I moved that code there a lot of other code sort of clicked into place. It also replaced the core/moduleconfig package
Also @pine plover could that generator.Introspect call just be given a client created by dagger.Connect(ctx)? 🤔
it used to handle nil by creating its own *engine.Client, so I don't think that introduces a new dependency or anything; it's higher-level now (*dagger.Client instead of *engine.Client)
pine plover
Also <@949034677610643507> could that `generator.Introspect` call just be given ...
Oh... yah... probably 😆
I'm blaming my residual jetlag! 
sand horizon
lolol