https://kotlinlang.org logo
Join Slack
Powered by
Is `result` missing in `Job` ? I'm trying to <rewr...
# github-workflows-kt
l
Is 
result
missing in 
Job
? I'm trying to rewrite this in Kotlin, and I don't find how to do so properly.
p
yes, it's missing from somewhere, but not sure if in 
Job
since there's 
needs.
in front. It's something new to me, it's in fact an unsupported context. As a short-term approach, try 
needs.${yourJob.id}.result
. Long-term, please create a feature request to add support for 
needs
context (so far we support these contexts).
v
Job indeed feels like the right place from a first look.
p
adding 
result
- probably yes, should be added to 
Job
to return an expression 
"${this.id}.result"
. @louiscad do you fancy adding it?
l
Yup, I can send a PR. Also, can I add Action Bindings for the 
nexus-actions
you see referenced in what I originally linked (in a separate PR)?
p
sure!
v
should be added to 
Job
to return an expression 
"${this.id}.result"
. @louiscad do you fancy adding it?
Not 
needs.${this.id}.result
? Can you otherwise refer to the job result besides within 
needs
?
p
not sure here if 
needs
should be implicit
I can imagine scenarios where we use 
job.result
in other contexts than 
needs
l
Don't you need 
needs
for all use cases where another job is referred to?
v
That was exactly my question 🙂
Also, can I add Action Bindings for the 
nexus-actions
You should also try to make them add action typings too in that action, so that the typings are not needed to be maintained here.
👀 1
l
Let's summon @mbonnin for that
p
Don't you need 
needs
for all use cases where another job is referred to?
I can imagine a case where I want to access e.g. jobs ID somewhere in the logic
m
for yaml ? 😅
p
so I'd prefer to keep 
needs
explicit for now
l
That said, I don't expect those actions to ever be touched again, so I'm not sure it's worth the complication. Maven Central / Nexus bureaucracy hasn't changed in years and years, probably since the beginning.
v
I can imagine a case where I want to access e.g. jobs ID somewhere in the logic
But that would be 
job.id
, wouldn't it? Not 
job.result
which then is 
${job/this.id}.result
That said, I don't expect those actions to ever be touched again, so I'm not sure it's worth the complication.
It is, it is not only meant for this action, but also as documentation for users and input for other automation tools yet to come 🙂
for yaml ? 😅
@mbonnin yes, a simple additional yaml that defines the types for inputs and outputs of the action
And optionally but recommended an additional workflow job that verifies the validity and consistency
m
Ah, that's very cool, I'm surprised this isn't a first party thing actually
v
We too 🙂
But it is hard to get it built-in in a timely manner so Piotr came up with it
💙 1
m
Created this for tracking https://github.com/nexus-actions/create-nexus-staging-repo/issues/7. I'm a bit under the waters right now but we'll add that eventually
v
Would also be nice to finally have 1st level support for alternate DSLs, so that you don't need to generate the YAMLs from the Kotlins with this library but it would happen automatically on run
💯 1
Created this for tracking https://github.com/nexus-actions/create-nexus-staging-repo/issues/7. I'm a bit under the waters right now but we'll add that eventually
As @louiscad wants to have it, maybe he will also come up with a PR for your repo
He has to do the work anyway if he wants it here without the typings in your action
🙂
l
@mbonnin I can submit the PRs while you're sous l'eau. Will you have some time to review, merge & release? I'll do my best to avoid errors so review is aussi simple que possible.
🫧 1
👈🏼 1
m
Sounds like a plan!
👌🏼 1
v
❤️
p
we'll have another action with typings, yay!
l
I'm also thinking about making higher level stuff going forward, so one can call a function, easily change a few parameters, and get a full workflow for complicated stuff like publishing to Maven Central
p
do you mean adding such workflow builders to the lib, or having it in your project?
regarding having 
needs
- I think what we need to check is if it's possible to use 
my-job-id.result
alone, without 
needs.
in front. I'm busy right now, happy to revisit it in the evening - @louiscad could you create a ticket so that we keep track of it?
l
In a dedicated project, be it in the Splitties GitHub organization, or in typesafegithub.
Regarding 
needs
, I'm quite sure it doesn't work if omitted, but I will double-check.
👍 1
p
please also check a case where you print out 
job-id.result
in some job's step
👍 1
FYI regarding the typings, I'm planning to decouple the typings stored outside of actions from the lib itself, thanks to which hopefully it will be easier for folks not really into Kotlin to contribute there (tracking ticket: #1072)
👍 1
v
> regarding having 
needs
- I think what we need to check is if it's possible to use 
my-job-id.result
alone, without 
needs.
Yes, that was exactly my question, and my guess is, that it is not. Because if you don't 
needs
it, it is also not guaranteed that it run before and finished already. And within the same job it also does not make sense. But we will see what Louis' experiments show. > I'm also thinking about making higher level stuff going forward, so one can call a function, easily change a few parameters, and get a full workflow for complicated stuff like publishing to Maven Central If you need some inspiration from excessive usage, you can have a look at https://github.com/Vampire/setup-wsl/blob/master/.github/workflows/test.main.kts 🙂
😁 1
👀 1
l
Mother-Of-God.jpg
😅 1
I don't imagine having to check that as YAML 😨
p
@Vampire you should finally update the lib to 1.x
💥 1
v
Yes, probably, but hey, it works. 😄
kodee broken hearted 1
I don't imagine having to check that as YAML 😨
I had it in YAML before @louiscad, but also with a preprocessing step due to the lack of anchor support in GitHub's YAML implementation: https://github.com/Vampire/setup-wsl/commit/4eea7c92d6c56a39bfcebcdcb93be63388497f59#diff-e79daa51dba0aab57ebc5872b[…]bfb9d609beb0757b4373e2e2a6b9c8
l
v
How do we have it for step results?
👀 1
l
An enum is probably not so ideal since it's not possible to override 
==
and 
!=
without a compiler plugin
Only ``==`` and ``!=`` are possible as infix functions (which is what I did for a project with a similar expression thing).
v
You don't actually compare in Kotlin anyway, so that shouldn't be relevant?
Ah, no we miss outcome for steps also, I just did it with an extension function
l
It's about how typesafe and how cumbersome it is.
v
You could tackle that alongside 🙂
l
Not really
v
😭
l
That would break the API alongside
But it can be interesting to investigate the options with Piotr
v
I meant adding step outcome context while adding job result context 😄
👍 1
For the operator thing @Piotr Krzemiński should weigh in, yes
l
I don't like how everything is a String in there, and a solution is probably possible
➕ 1
v
As long as you can also just get the context reference string like I need it in that test workflow I linked. 🙂
l
I was thinking make a 
Result
class and have 
isSuccess()
, 
isNotSuccess()
, 
wasSkipped()
, 
wasNotSkipped()
, etc
That would not require an awkward ``==`` or ``!=``.
v
As long as it is flexible enough to also be used low-level as GitHub also likes to change this sometimes, or maybe add some status, or you might need it outside a 
condition
like here: https://github.com/Vampire/setup-wsl/blob/master/.github/workflows/test.main.kts#L241
👍 1
l
NEQ, EQ could work
An alternative naming to the "awkward" I mentioned just above
v
But that's ugly 😄
p
Sorry, too much topics in this thread :D open to changes, but how about having separate threads?
(will read in the evening)
👍 1
👌 1
l
Still the same topic
It's only unraveling
v
Not really many topics, just about how to nicely do outcome / result
p
Ah, ok
I'm generally all for making the expressions more type-safe. What we have now is essentially using strings with elements of type-safety, and I'm aware it's not ideal. It is, however, still better than YAML 😁
I thought of a proper expressions overhaul, can have breaking changes. I'm tentatively planning to release 2.0 in Q1 or Q2 2024, together with switching to client-side binding generation, so it's a good chance to introduce other breaking changes
Let me know if you have concrete ideas. Best to gather them in an issue, here's a related one https://github.com/typesafegithub/github-workflows-kt/issues/372
l
p
thanks, enqueued
v
infix didn't work out?
l
I didn't try. You want infix ``||`` & ``!=`` or just make 
eq
and 
neq
infix? @Vampire
v
🤷‍♂️
l
Let's try 
eq
and 
neq
for now
m
Thanks for the PR @louiscad! @Piotr Krzemiński is a new tag required for the action for the generator to pick up the typings? Is the DSL generator version-aware ?
l
I need to fix a few things @mbonnin, sorry I didn't check in my fork before sending the PR
m
No worries at all
v
Yes, at least in the current state you can have v1, v2, ... versions for the action. Hopefully you have a vX tag or branch that points to the release and not only vX.Y.Z, otherwise the action disqualifies anyway 😄
m
@Vampire yea, we haven't been the most rigourous in our release process so far so we have only tags and no specific convention 😕
I was thinking taging a 
v1.3
. Is that important to 
github-workflows-kt
?
l
The 
v1
tag should probably be renamed to 
v1.0
and 
v1
recreated to point to whatever the latest is
👍 1
m
I'm against branches in GitHub actions
Well maybe not so strong as "against" but don't really see point
l
I was talking tags @mbonnin
m
👍 was just realizing that 🙂
Agreed 👍
l
@mbonnin Can you undo my commit so I can re-send a proper PR?
m
just redo a pr on top of main?
I'm against force pushes to main 😄
p
m
Oh my... Is GitHub actually recommending referencing a moving tag? Isn't that breaking build reproducibility 100%?
(sorry for side tracking this discussion completely, I don't mind that much and I won't be dying on this hill. I just find it ... surprising)
l
GitHub acquired npm…
And was then acquired by Microsoft, and we all know their flagship product Windows is well known for reproductibility
💯 1
m
I mean they came up with yaml so I shouldn't be surprised I guess 😂
l
Exactly.
I think build reproducibility is a noble target, and that's probably something I want to chat with @Piotr Krzemiński at some point. Right now, it's way too cumbersome, because updating the workflows is cumbersome. Something can probably done, but talking about that further would definitely be bike-shedding 😅 BTW, I sent another PR : https://github.com/nexus-actions/create-nexus-staging-repo/pull/9
🙏 1
merged 1
m
updating the workflows is cumbersome
It's a pet peeve of mine 😄
👀 1
l
Nice!
Already starred 🙈
😄 1
Maybe a post-processing step on the YAML generation could be done to integrate your pinning
m
Did some shuffling in tags and releases. Tags should be all good now: https://github.com/nexus-actions/create-nexus-staging-repo/releases/tag/v1.3.0
❤️ 1
l
m
Need to go AFK for ~30min, will do the merging and release in a bit
❤️ 1
l
@mbonnin I think you're missing the 
v1.3
tag
v
I'm against branches in GitHub actions
Oh my... Is GitHub actually recommending referencing a moving tag? Isn't that breaking build reproducibility 100%?
@mbonnin exactly and imho that is a total no-go. That's why I do not have a rolling tag for the major version, but a branch for the major version and made Piotr accept that for this action too. 🙂 I also don't like having build results in VCS. So I indeed have a branch for the major version and on release merge 
master
into that branch, and add the build artifacts only in that branch which is at least a bit cleaner imho than a rolling tag and having the build artifacts in the normal branches. You can have a look at https://github.com/Vampire/setup-wsl if you like 🙂
m
I think you're missing the 
v1.3
tag
Is that useful? I only kept 
v1.1
and 
v1.2
for backward compat but I don't think they should be actually used anymore. Just use the path version 
vx.y.z
or the current major 
v1
. Is there a need for the current minor as well?
v
Depends on your users, and / or your backwards compatibility policy 🙂
l
Oh I see, maybe it's not needed and v1 is enough, yes
m
backwards compatibility policy is semver
v
Then probably not
👍 1
m
a branch for the major version and made Piotr accept that for this action too
nice! I switched to that as well
👌 1
v
If I one day start a blog finally, I should write a post about how to properly version GitHub actions 😄
nod 1
💯 1
m
Alright, last 2 PRs done on my side
And all PRs should be merged and tags/branches pushed. Hope I didn't mess things up 🤞
❤️ 1
p
Sorry for being not really active here, crazy time here! I see everything's going forward nicely :D
👍 1
Regarding tags/branches: from PoV of the bindings generator, it doesn't care what it is, it just needs to be a valid git ref using the pattern 
vN
where 
N
is an integer :) in the world of client-side binding generation (currently experimental) it will be possible to consume any action version, even a concrete commit hash, like in a pure YAML workflow
👍🏼 1
👍 1
l
Alright, my 2PRs are there, your move ♟️ 😄
♟️ 2
p
Thanks! Will review soon-ish
l
@Piotr Krzemiński I searched and reviewed the GitHub Workflows documentation, and needs is the only thing there for 
result
of a 
job
, and my previous workflow runs in publishing (or failing to) several projects to Maven Central confirm it is working as documented, so I see no point in trying a made-up, undocumented way. However, one interesting thing would be to check that the job looking at the result of another job indeed depends on said former job (via 
needs
) when generating the YAML. If you have ideas on how to implement this, let me know, but what we have in the PR is already better than the status quo IMO.
👍 1
p
@louiscad sorry, I didn't get what you meant here https://kotlinlang.slack.com/archives/C02UUATR7RC/p1700233748033889?thread_ts=1700111531.986899&amp;cid=C02UUATR7RC - could you rephrase, give more context?
Do you mean referring to action versions by major versions instead of being able to specify a concrete commit hash?
If yes, you kinda can achieve it already. Just pass another argument when building the binding instance: _customVersion. It will be used as a string after @ in the YAML
l
Upgrading is a bit cumbersome though, and commit hashes are less friendly than version numbers. So, yes, github-workflows-kt already supports version pinning, and yet, it's only part of the story to make it easy.
p
Client-side bindings generation will eventually let you refer to any ref of a given action, let it be a tag, a branch or commit hash :) Currently this mode supports only major version tags/branches
3 Views
Open in Slack
PreviousNext
Powered by