Skip to content

Conversation

@rswanson
Copy link
Member

@rswanson rswanson commented Jun 3, 2025

No description provided.

@rswanson rswanson mentioned this pull request Jun 3, 2025
@rswanson rswanson requested review from a team, Evalir and dylanlott June 3, 2025 14:39
@rswanson rswanson force-pushed the swanny/i-need-more-validation branch from bd6d961 to af1fc29 Compare June 3, 2025 14:39
@rswanson rswanson requested a review from Copilot June 3, 2025 14:40
@rswanson rswanson force-pushed the swanny/i-need-more-validation branch from af1fc29 to 312ca45 Compare June 3, 2025 14:40
@rswanson rswanson force-pushed the swanny/release-workflow branch from 548a2a7 to ffc61e3 Compare June 3, 2025 14:40
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

A broad refactor that adds a manual GitHub Actions–driven release workflow and centralizes constants, validation, and utility functions across multiple packages.

  • Introduce a release.yml workflow with workflow_dispatch for manual releases.
  • Extract shared defaults (ports, labels, resource requirements) into constants and utils helpers.
  • Expand and enforce validation for SignetNode, Quincey, Builder, and AWS IAM components with updated tests and type tags.

Reviewed Changes

Copilot reviewed 16 out of 16 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
.github/workflows/release.yml Add manual-triggered release workflow
pkg/utils/env.go Add ParsePortWithDefault, CreateResourceRequirements, CreatePersistentVolumeClaim
pkg/signet_node/types.go Add validate:"required" tags, update SignetNodeEnv struct
pkg/signet_node/validation.go Expand SignetNodeComponentArgs and SignetNodeEnv validation
pkg/signet_node/validation_test.go Update tests for new SignetNode validation rules
pkg/quincey/constants.go Extract Quincey defaults and identifiers
pkg/quincey/types.go Remove inline constants, rely on shared constants.go
pkg/quincey/quincey.go Replace literals with constants, use utils.ParsePortWithDefault
pkg/quincey/validation_test.go Adjust Quincey component args tests
pkg/builder/constants.go Define Builder defaults and suffix constants
pkg/builder/types.go Add validate:"required" tags to BuilderEnv fields
pkg/builder/validation.go Expand BuilderEnv.Validate for all required fields
pkg/builder/validation_test.go Update builder tests to match new validation order
pkg/builder/builder.go Replace literals and inline probes with constants and helpers
pkg/aws/constants.go Introduce IAM policy/version constants
pkg/aws/iam.go Refactor IAM resource creation to use constants
Comments suppressed due to low confidence (4)

pkg/signet_node/types.go:64

  • There's a typo in the Pulumi tag for HostStartTimestamp (hostStartTimepstamp); it should be hostStartTimestamp to match the field name.
HostStartTimestamp            pulumi.StringInput `pulumi:"hostStartTimepstamp" validate:"required"`

pkg/signet_node/types.go:51

  • The RustLog field lacks a validate:"required" tag but is checked in SignetNodeEnv.Validate; add the tag for consistency.
RustLog                       pulumi.StringInput `pulumi:"rustLog"`

pkg/signet_node/validation_test.go:67

  • This test (invalidArgs1) omits Name and will fail on name is required first; update the expected message to "name is required".
assert.Contains(t, err.Error(), "namespace is required")

pkg/utils/env.go:151

  • There are no unit tests covering ParsePortWithDefault; adding tests for valid and invalid port strings would improve coverage.
// ParsePortWithDefault converts a port string to an integer with a default fallback

@rswanson rswanson changed the base branch from swanny/i-need-more-validation to graphite-base/13 June 3, 2025 14:43
@rswanson rswanson force-pushed the graphite-base/13 branch from 312ca45 to bf5093d Compare June 3, 2025 14:44
@rswanson rswanson force-pushed the swanny/release-workflow branch from ffc61e3 to 0f65a84 Compare June 3, 2025 14:44
@graphite-app graphite-app bot changed the base branch from graphite-base/13 to main June 3, 2025 14:44
@rswanson rswanson force-pushed the swanny/release-workflow branch from 0f65a84 to 07f6914 Compare June 3, 2025 14:44
@rswanson rswanson merged commit 89f1462 into main Jun 3, 2025
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants