#Opt changes

1 messages · Page 1 of 1 (latest)

limpid mirage
#

move the threat to not creating noise in the channel

#

I guess, tags most common way to do it but totally agree that it's confusing working differently inside and outside of the module. I guess we could always add a type similar to Optional like Configuration[T Configurable] and move the previous logic there.

#

It should be not make difference for the engine like Optional

sinful urchin
#

Ahhhhhh yes I really want to have a generic like that 😁
The problem is you can't have values passed as type parameters in go (like you can with const generics in rust)

limpid mirage
#

Ahh that limitation, it's most annoying thing about go generics

tall widget
#

wait didn't we keep the support for anonymous structs, which includes support for default/required args? haven't actually checked post-merge

sinful urchin
#

anonymous struct yes, i did disconnect the default args though

#

i can hook them up again though if we need to

tall widget
#

I suppose the idea is to avoid a split experience between the two?

#

i.e. now I want a default so I gotta refactor to the anonymous struct approach

sinful urchin
#

yeah, i'd like to avoid that i think

#

we couuuld always add it in if we need to - but i'd temporarily like to resist to see how much of an issue it really is, to reconnect to the defaults stuff, it's just a little struct lookup, i've kept the json unmarshalling logic

#

you'd just need to write to that field

tall widget
#

well - at that point we might as well just not have inline struct support, since the only thing it lets you do is write your code in a weird way, no? all stick, no carrot

sinful urchin
#

i'm not opposed to reconnecting up the defaults - i'd push back on optional stuff though, and say we should be consistent with Optional

#

or we could happily support all of it as before, but i'll be honest, not sure i can flesh that out today

tall widget
#

oh yeah no worries, if I find time I can look into it, will keep chewing on the thought. thanks for clarifying

#

it'll be good to see who comes out of the woodwork and misses it besides me, in the meantime

sinful urchin
#

i think we got crossed-wires at some point - i missed that we wanted to keep defaults for the struct syntax, sorry!

tall widget
limpid mirage
#

Just as feedback, initially, I was horrified when I saw this change since I was configuring too many options on multiple levels. However, I need to admit without opts structs code is simpler to understand. 👍

limpid mirage
#

I'll create bug in the meantime let me post here as well.

func (ar *ActionRun) WithInput(name, value string) *ActionRun {
    ar.Config.With = append(ar.Config.With, fmt.Sprintf("%s=%s", name, value))
    return ar
}

this code block causing a code gen error.

dagger mod sync
✘ asModule(sourceSubpath: "daggerverse/actions/runtime") ERROR [3.18s]
✘ exec /usr/local/bin/codegen --module . --propagate-logs=true ERROR [0.39s]
┃ Error: generate code: template: module:56:3: executing "module" at <Modu
┃ MainSrc>: error calling ModuleMainSrc: runtime error: index out of range
┃ 1] with length 1
┃ Usage:
┃   codegen [flags]
┃
┃ Flags:
┃   -h, --help             help for codegen
┃       --lang string      language to generate (default "go")
┃       --module string    module to load and codegen dependency code
┃   -o, --output string    output directory (default ".")
┃       --propagate-logs   propagate logs directly to progrock.sock
┃
┃ Error: generate code: template: module:56:3: executing "module" at <Modu
┃ MainSrc>: error calling ModuleMainSrc: runtime error: index out of range
┃ 1] with length 1
✘ generating go module: actions-runtime ERROR [0.28s]
• Engine: d8c37de1d5af (version devel ())
⧗ 4.89s ✔ 238 ∅ 2 ✘ 3
Error: failed to automate vcs: failed to get vcs ignored paths: input:1: host.directory.asModule failed to call module "actions-runtime" to get functions: failed to get function output directory: process "/usr/local/bin/codegen --module . --propagate-logs=true" did not complete successfully: exit code: 1
#

it's not a big deal but it would be good to add docs if it's not already mentioned

sinful urchin
#

confirmed, investigating salute

limpid mirage
sinful urchin
#

almost have a fix for ya ❤️ thanks so much ❤️