-
Notifications
You must be signed in to change notification settings - Fork 26
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Dagger for GitHub action for Dagger v0.9.3 #98
Conversation
Signed-off-by: Jeremy Adams <[email protected]>
Signed-off-by: Kyle Penfound <[email protected]> Signed-off-by: Jeremy Adams <[email protected]> Co-authored-by: Jeremy Adams <[email protected]>
1b2f8c2
to
56c183f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I love this as a way to use Zenith.
One important question: can this also used as a wrapper to dagger run
in pre-Zenith CLI? And, should it?
This comment was marked as resolved.
This comment was marked as resolved.
Yeah, we need to make sure the UX is smooth, but making this dual usage would be great. |
834f53c
to
9e4beb8
Compare
Pretty good for |
action.yml
Outdated
DAGGER_VERSION=${{ inputs.version }} sh 2>/dev/null; } | ||
shell: bash | ||
|
||
- run: if ! [ -d ./.git ]; then git clone -b ${GITHUB_REF_NAME} --no-checkout https://github.com/${GITHUB_REPOSITORY} .; fi |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this so we can get the git metadata for cloud?
Makes sense for now if so, but one of the cool benefits is that the git checkout should be optional now. With some updates to how we get the git metadata (i.e. optionally parse from env rather than the local .git
) we could achieve that I think. Obviously not a blocker here, just checking that's the reason we included this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll try it without this and see if it's really needed for Dagger Cloud.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I don't see any reason to keep it. It's outta here!
Signed-off-by: Jeremy Adams <[email protected]>
Signed-off-by: Jeremy Adams <[email protected]>
9e4beb8
to
ea5581f
Compare
Signed-off-by: Jeremy Adams <[email protected]>
We'll release this as
v5.0.0
,v5
https://docs.github.com/en/actions/creating-actions/about-custom-actions
Works as expected here for
dagger call
anddagger run
cases:https://github.com/jpadams/test-new-action/actions/runs/6859800772/job/18652631258
DX is basic, but pretty good for both
dagger run
anddagger call
.https://github.com/jpadams/test-new-action/blob/main/.github/workflows/run-and-call.yml#L27-L41
Updated
README.md
:https://github.com/dagger/dagger-for-github/blob/ea5581fc7d79ae81db8c18029cbc76d3f45cf1ea/README.md