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

actions: keep scripts out of the workflow scope #6168

Closed
gustavovalverde opened this issue Feb 15, 2023 · 8 comments · Fixed by #8005
Closed

actions: keep scripts out of the workflow scope #6168

gustavovalverde opened this issue Feb 15, 2023 · 8 comments · Fixed by #8005
Assignees
Labels
A-devops Area: Pipelines, CI/CD and Dockerfiles A-infrastructure Area: Infrastructure changes C-enhancement Category: This is an improvement S-needs-triage Status: A bug report needs triage

Comments

@gustavovalverde
Copy link
Member

Motivation

Some workflows are script intensive, making the workflows unreadable.

Specifications

A good approach to handle this would be moving all the scripts to multiple files outside the workflow, or create a GitHub Action to execute them in a reusable manner.

Complex Code or Requirements

  • Define a place to hold all the script files
  • Keep linting working on these files
  • Use a name convention for script files
@gustavovalverde gustavovalverde added A-infrastructure Area: Infrastructure changes A-devops Area: Pipelines, CI/CD and Dockerfiles C-enhancement Category: This is an improvement S-needs-triage Status: A bug report needs triage P-Low ❄️ labels Feb 15, 2023
@gustavovalverde gustavovalverde self-assigned this Feb 15, 2023
@mpguerra mpguerra added this to Zebra Feb 15, 2023
@github-project-automation github-project-automation bot moved this to 🆕 New in Zebra Feb 15, 2023
@teor2345
Copy link
Contributor

The other advantage of this is that it's easier to test the scripts locally outside the workflows.

@mpguerra
Copy link
Contributor

@teor2345
Copy link
Contributor

This is potentially a very large task, depending on how much work we put into de-duplicating or parameterising our scripts.

What's the intended scope of this ticket? Is it just moving the existing scripts into files? Or will there be a lot of refactoring?

(CI refactoring is risky and often breaks things, because it's difficult to test before merging.)

@gustavovalverde
Copy link
Member Author

Is it just moving the existing scripts into files? Or will there be a lot of refactoring?

It's basically moving scripts into .sh files, but just doing that can be considered a lot of refactoring.

@teor2345
Copy link
Contributor

teor2345 commented Aug 20, 2023

Is it just moving the existing scripts into files? Or will there be a lot of refactoring?

It's basically moving scripts into .sh files, but just doing that can be considered a lot of refactoring.

I guess what I meant was: are we just moving the script text into files and calling those files? Or are we modifying the functionality of the scripts at the same time?

I think it would reduce the risk and scope of this ticket to restrict it to copy-pasting scripts, and also do it in stages. Maybe we could try it with a small workflow or a single script first, to check if there are any issues?

@gustavovalverde
Copy link
Member Author

Yeah, it should basically be copy-pasting, and we can surely test with the less criticals first. I've anyways remove this from the Sprint backlog as we have these dependencies:

@teor2345
Copy link
Contributor

teor2345 commented Jan 2, 2024

In PR #8132 I reverted this PR because it has been causing internal GitHub errors. Unfortunately no-one picked up on this during the review process or after it merged.

@teor2345 teor2345 reopened this Jan 2, 2024
@teor2345
Copy link
Contributor

teor2345 commented Jan 2, 2024

When we re-do this PR, we need to manually go to the ci-integration-tests-gcp.yml workflow run, scroll down, and check it for errors. These errors don't reliably show up in the CI summary or auto-opened tickets.

For example, here is a successful run:
https://github.com/ZcashFoundation/zebra/actions/runs/7188577552?pr=8080

And here is a failed run:
https://github.com/ZcashFoundation/zebra/actions/runs/7381289342

@mpguerra mpguerra moved this from Done to In progress in Zebra Jan 16, 2024
@github-project-automation github-project-automation bot moved this from In progress to Done in Zebra Jan 29, 2024
@mpguerra mpguerra reopened this Jan 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-devops Area: Pipelines, CI/CD and Dockerfiles A-infrastructure Area: Infrastructure changes C-enhancement Category: This is an improvement S-needs-triage Status: A bug report needs triage
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants