-
Notifications
You must be signed in to change notification settings - Fork 0
gha #35
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
base: master
Are you sure you want to change the base?
gha #35
Conversation
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.
Pull request overview
This PR introduces a GitHub Actions CI/CD workflow for a Clojure project. The workflow automates building, testing, and publishing the project to Clojars.
Key Changes
- Adds a comprehensive CI pipeline that runs on all branches and pull requests
- Implements conditional testing and deployment jobs based on branch/tag context
- Configures Java 17 and Leiningen for Clojure project builds
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
.github/workflows/ci.yml
Outdated
| - name: Install dependencies | ||
| run: | | ||
| sudo apt-get update | ||
| sudo apt-get install -y gzip |
Copilot
AI
Dec 23, 2025
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 step name 'Install dependencies' is duplicated multiple times in the workflow with different purposes. This step installs system packages (gzip), while later steps with the same name install Leiningen dependencies. Consider renaming this to 'Install system dependencies' to clarify its purpose and distinguish it from other dependency installation steps.
.github/workflows/ci.yml
Outdated
| - name: Install Leiningen | ||
| run: | | ||
| wget https://raw.githubusercontent.com/technomancy/leiningen/stable/bin/lein | ||
| chmod +x lein | ||
| sudo mv lein /usr/local/bin/ | ||
| lein version |
Copilot
AI
Dec 23, 2025
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 Leiningen installation steps are duplicated across all three jobs (build, test, and publish). Consider extracting this into a reusable composite action or using a pre-built action from the marketplace to reduce duplication and improve maintainability.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
colinphill-mdsol
left a comment
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.
Thanks @janitmodi for getting this started. Leaving some more comments for whoever carries this over the finish line.
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.
All actions should be pinned to a specific commit using ratchet. This is something infosec has requested for all GHA.
| - name: Install system dependencies | ||
| shell: bash | ||
| run: | | ||
| sudo apt-get update | ||
| sudo apt-get install -y gzip |
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 don't think this is needed
| install-deps: | ||
| description: 'Whether to install Leiningen dependencies (lein deps and lein pom)' | ||
| required: false | ||
| default: 'false' |
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.
This appears to be unused.
| - name: Install Leiningen | ||
| uses: DeLaGuardo/setup-clojure@13.0 | ||
| with: | ||
| lein: 'latest' |
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.
Versions must all be pinned.
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.
Again, action versions should be pinned with ratchet.
No description provided.