Skip to content

Improve CICD#98

Open
adrian-brady wants to merge 1 commit intoreorganizefrom
cicd
Open

Improve CICD#98
adrian-brady wants to merge 1 commit intoreorganizefrom
cicd

Conversation

@adrian-brady
Copy link
Contributor

No description provided.

@adrian-brady adrian-brady marked this pull request as ready for review May 7, 2025 22:13
@adrian-brady adrian-brady changed the base branch from main to reorganize May 7, 2025 22:15
Copy link
Contributor

@sarthyparty sarthyparty left a comment

Choose a reason for hiding this comment

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

hopefully my review helps

@@ -0,0 +1,30 @@
name: Lint
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: add linter for yaml code, especially since there are many formatting inconsistencies

@@ -0,0 +1,139 @@
name: Main CI Pipeline

# on:
Copy link
Contributor

Choose a reason for hiding this comment

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

why is this commented?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

so it doesnt run. i have zero confidence in my abillity to fix things and this is my first time messing with GH workflows/actions and I dont wanna kill prod


on:
pull_request:
# on:
Copy link
Contributor

Choose a reason for hiding this comment

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

why is this commented?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is the old implementation of the build and test procedure. I wanted to keep it around for reference but not have it run

echo "Running Flyway migrations on server..."
echo "Current Directory: $(pwd)"

if [ -z "$DB_HOST" ] || [ -z "$DB_USER" ] || [ -z "$DB_NAME" ] | [ -z "$DB_PASSWORD" ]; then
Copy link
Contributor

Choose a reason for hiding this comment

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

why a single | in the last or statement? is that a bug?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes bug. I havent tested this because it gets run on prod and i dont really know whats going on with flyway/db migrations

@sarthyparty
Copy link
Contributor

@adrian-brady Nit: common steps—checkout, Go setup, lint/test, SQLC vet/diff, and build—are repeated across several workflows; let’s consolidate them into fewer workflows or a shared composite action.

@adrian-brady
Copy link
Contributor Author

@adrian-brady Nit: common steps—checkout, Go setup, lint/test, SQLC vet/diff, and build—are repeated across several workflows; let’s consolidate them into fewer workflows or a shared composite action.

could you help me with that?

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.

2 participants