Skip to content

Conversation

@codygordon
Copy link
Contributor

@codygordon codygordon commented Jul 30, 2025

Travis CI account is cancelled so the auto-deploy wouldn't work. We haven't merged a change here for exactly a year, so this isn't super urgent, just need it merged when we plan to deploy next.

The packages are so out of date that updating them might have unexpected side effects, so I wanted to make minimal changes here to avoid regressions. That said, I did change some things for consistency:

  • Node 10.x is used throughout.
  • npm ci is used and the package-lock.json is correctly committed.
  • Gulp commands are run via script commands or npx so they always use the defined verions.
  • Python 3.10 needs to be installed, otherwise the npm install step in the workflow will throw errors when trying to install the older version of node-gyp used under the hood for a couple packages.
  • Clarified the PR runner for testing the build and set it to use the same setup/commands as the deploy runner

I've set up a test branch that runs the deployment to test bucket directories, see the runner results:
https://github.com/MoveOnOrg/giraffe/actions/runs/16627415641/job/47047602463

We can view individual assets directly to verify their correctness, i.e.:
https://static.moveon.org/test/styles/main.css

or list them via CLI:
aws s3 ls s3://static.moveon.org/test/

@codygordon codygordon requested a review from Copilot July 30, 2025 15:47
Copy link

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

This PR migrates the auto-deployment system from Travis CI to GitHub Actions workflows, maintaining the same deployment functionality while updating the tooling and documentation to reflect modern best practices.

  • Replaces Travis CI configuration with GitHub Actions workflows for both build checks and deployment
  • Updates Node.js version management and build commands to use npm scripts consistently
  • Modernizes documentation to reflect the new GitHub Actions-based deployment process

Reviewed Changes

Copilot reviewed 8 out of 10 changed files in this pull request and generated no comments.

Show a summary per file
File Description
package.json Adds npm scripts for compile and watch commands to standardize build process
README.md Updates deployment documentation to reference GitHub workflows instead of Travis
CONTRIBUTING.md Modernizes development setup instructions with nvm usage and npm scripts
.travis.yml Removes Travis CI configuration (entire file deleted)
.nvmrc Simplifies Node version specification from specific to major version
.github/workflows/npm-gulp.yml Removes old GitHub workflow file
.github/workflows/deploy.yml Adds new deployment workflow with S3 sync capabilities
.github/workflows/build-check.yml Adds build verification workflow for pull requests
Comments suppressed due to low confidence (1)

.github/workflows/deploy.yml:44

  • The secret name 'AWS_ACCESS_KEY_SECRET' is inconsistent with the typical GitHub convention. Consider renaming to 'AWS_SECRET_ACCESS_KEY' to match the standard AWS credential naming pattern.
          aws-secret-access-key: ${{ secrets.AWS_ACCESS_KEY_SECRET }}

@codygordon codygordon requested a review from stvfltchr July 30, 2025 19:24
@codygordon
Copy link
Contributor Author

codygordon commented Jul 30, 2025

@stvfltchr some background on this repo:

  • It was created by contractors who were responsible for the 2018 rebrand, which we still use today.
  • It was meant to contain the complete organizational styleguide site and set of components, but it was never finished.
  • The "giraffe" name is just a random silly name someone picked and refers specifically to these assets / styles.
  • The css is based on Bootstrap and is compiled from sass.
  • Today it is mainly used to house and deploy brand assets and the main stylesheet used in several places (front, AN, maybe more): https://static.moveon.org/giraffe/styles/main.css

If we want to make changes to that css they need to be made to the sass here, compiled, and this deploy job would need to be triggered.

I don't know how much we want to invest in improving (or totally reworking) this repo, but it is one of those non-urgent things we probably should consider looking into at some point.

Copy link

@stvfltchr stvfltchr left a comment

Choose a reason for hiding this comment

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

This is approved. @codygordon, since this is my first PR approval, let's grab a minute at some point so you can show me how to verify that this deployment succeeded and the files are getting where they're supposed to go with the new workflow.

@codygordon
Copy link
Contributor Author

Sounds good! I can find some time. Merging this into main would re-deploy the assets, so I think we want to be super sure they are exactly how we expect them to be. We can use the test results to diff what is currently deployed to make sure things look good, then merge while pairing and keep an eye on it to make sure it worked as expected and nothing weird is going on with the live sites as a result.

@codygordon codygordon merged commit e208d74 into main Aug 1, 2025
1 check 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