-
Notifications
You must be signed in to change notification settings - Fork 214
Fe overhaul managed compute and onboarding #1614
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
Conversation
Bumps [k8s.io/client-go](https://github.com/kubernetes/client-go) from 0.32.3 to 0.33.0. - [Changelog](https://github.com/kubernetes/client-go/blob/master/CHANGELOG.md) - [Commits](kubernetes/client-go@v0.32.3...v0.33.0) --- updated-dependencies: - dependency-name: k8s.io/client-go dependency-version: 0.33.0 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
@@ -0,0 +1,29 @@ | |||
name: Sync Go Quickstart | |||
|
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.
Can we have a single action for these that loops through the three or something? looks like these are all the same
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.
the on paths is different for each, i wanted one too but i dont want to push changes to all if only 1 changes
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.
womp womp
value={selectedWorkflowId} | ||
onChange={(e) => { | ||
setSelectedWorkflowId(e.target.value); | ||
setSelectedRunId(''); // Reset selected run when workflow changes |
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.
nit but something I really dislike about the AI-gen code 😅 I really dislike comments like this - it's obvious if you're reading the code what's going on here, and if it's not that's a problem with how the code is written IMO
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 agree
{!disabledCapabilities.includes('workflow') && ( | ||
<div> | ||
<label className="text-sm font-medium">Workflow</label> | ||
<select |
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.
should we be using shad components here?
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.
good catch
unknown: 'txt', | ||
}; | ||
|
||
// This is a server component that will be rendered at build time |
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.
do we have server components?
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.
its statically rendered at build time, maybe its not technically a server component
const currentStepIndex = activeStep ? stepKeys.indexOf(activeStep) : -1; | ||
|
||
useEffect(() => { | ||
console.log('activeStep', activeStep); |
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'm surprised these log lines aren't caught by the linter?
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.
me too
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.
left a couple comments but basically blindly approving 😅 tough to really review this
I have some qualms with the vibe coded stuff but I think it's fine to leave for now :P
Description
Adds onboarding and managed compute