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

[DEVOPS-6292] Migrate CICD from CircleCI to Github Actions #14

Merged
merged 2 commits into from
Aug 10, 2023

Conversation

anaharris
Copy link
Contributor

@anaharris anaharris requested a review from a team August 9, 2023 20:14
${{ runner.os }}-

- name: Run Rubocop
run: bundle exec rubocop --config ./.rubocop.yml

Choose a reason for hiding this comment

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

LGTM only thing is the no new line at the end of the file but that might just be a yaml linting thing

Copy link

@dantech2000 dantech2000 left a comment

Choose a reason for hiding this comment

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

LGTM 👍🏼

Copy link

@mbacchi mbacchi left a comment

Choose a reason for hiding this comment

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

Looks good, one minor comment but that shouldn't block you.

with:
bundler-cache: true

- name: Cache dependencies
Copy link

Choose a reason for hiding this comment

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

I wonder if it's even necessary to cache dependencies for a project like this which is built very rarely. (I'm also not sure how long the cache stays in place, and if it's purged automatically or if it costs money to maintain it in Github Actions' cache.)

I'll leave it up to you to decide Ana.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree, we probably don't need caching on this repo. No one has touched it in ages and afaik there aren't any updates to it happening soon. We can revisit but for now I'll remove caching.

@anaharris anaharris merged commit 6ac056f into main Aug 10, 2023
@anaharris anaharris deleted the DEVOPS-6292 branch August 10, 2023 21:19
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.

4 participants