Skip to content

gha#35

Open
jmodi-mdsol wants to merge 6 commits into
masterfrom
janit_travis_migration_2025_12_23
Open

gha#35
jmodi-mdsol wants to merge 6 commits into
masterfrom
janit_travis_migration_2025_12_23

Conversation

@jmodi-mdsol

Copy link
Copy Markdown

No description provided.

Copilot AI left a comment

Copy link
Copy Markdown

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.

Comment thread .github/workflows/ci.yml Outdated
Comment on lines +22 to +25
- name: Install dependencies
run: |
sudo apt-get update
sudo apt-get install -y gzip

Copilot AI Dec 23, 2025

Copy link

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.

Copilot uses AI. Check for mistakes.
Comment thread .github/workflows/ci.yml Outdated
Comment thread .github/workflows/ci.yml Outdated
Comment on lines +34 to +39
- 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

Copy link

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.

Copilot uses AI. Check for mistakes.
Comment thread .github/actions/setup-leiningen/action.yml Outdated
Comment thread .github/actions/setup-leiningen/action.yml Outdated
jmodi-mdsol and others added 4 commits December 23, 2025 11:51
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

@colinphill-mdsol colinphill-mdsol left a comment

Copy link
Copy Markdown

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.

Copy link
Copy Markdown

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.

Comment on lines +15 to +19
- name: Install system dependencies
shell: bash
run: |
sudo apt-get update
sudo apt-get install -y gzip

Copy link
Copy Markdown

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

Comment on lines +8 to +11
install-deps:
description: 'Whether to install Leiningen dependencies (lein deps and lein pom)'
required: false
default: 'false'

Copy link
Copy Markdown

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'

Copy link
Copy Markdown

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.

Comment thread .github/workflows/ci.yml

Copy link
Copy Markdown

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.

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