Skip to content
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

Add cd workflow with pip & conda deploy, etc. #27

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

charles-turner-1
Copy link
Collaborator

  • Setup environment.yml and meta.yml for conda deploy.

- Setup environment.yml and meta.yml for conda deploy.
Copy link

codecov bot commented Feb 4, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 82.77%. Comparing base (aa468af) to head (8c11288).

Additional details and impacted files
@@           Coverage Diff           @@
##             main      #27   +/-   ##
=======================================
  Coverage   82.77%   82.77%           
=======================================
  Files           7        7           
  Lines         360      360           
=======================================
  Hits          298      298           
  Misses         62       62           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Member

@CodeGat CodeGat left a comment

Choose a reason for hiding this comment

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

Just a couple of points! I only really reviewed the CD stuff :)

Comment on lines +17 to +20
- name: Set up Python 3.11
uses: actions/setup-python@v5
with:
python-version: 3.11
Copy link
Member

Choose a reason for hiding this comment

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

Although not super critical, you can cache the installation of later python packages by adding the cache: pip argument to the setup-python action, like so:

Suggested change
- name: Set up Python 3.11
uses: actions/setup-python@v5
with:
python-version: 3.11
- name: Set up Python 3.11
uses: actions/setup-python@v5
with:
python-version: 3.11
cache: pip

@@ -0,0 +1,62 @@
name: CD

on: [push, pull_request]
Copy link
Member

Choose a reason for hiding this comment

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

It running on.push without specifying any branches might be overly broad. Is this deployment to PyPi supposed to be for only pushes to main? If so, it can be updated to:

Suggested change
on: [push, pull_request]
on:
push:
branches:
- main
paths-ignore:
- .github # updates to CD infra shouldn't deploy to pypi
pull_request:
paths-ignore:
- .github # probably the case here, too

Comment on lines +36 to +37
needs: pypi
if: always() && needs.pypi.result == 'success'
Copy link
Member

Choose a reason for hiding this comment

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

Just so I understand, is it the intention that the conda job runs only on.pull_request, or both on.push/on.pull_request?

Currently it will only run on.push - on.pull_request won't run this job, because the status of the pypi job will be skipped rather than success. If you want it to run on.pull_request, the condition should be:

if: always() && (needs.pypi.result == 'success' || needs.pypi.result == 'skipped')

If you do want it to run only on.push, the conditional can be removed, as by default the job will only run if the jobs it needs are success.

@charles-turner-1
Copy link
Collaborator Author

Just a couple of points! I only really reviewed the CD stuff :)

Awesome, thanks! Not the most clued up with github actions - I copied most of this from the intake-catalog package.

I'll go through the workflows with your comments & see if I can actually understand whats going on from there.

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