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

Nested pathogen-repo-ci #65

Closed
wants to merge 3 commits into from
Closed

Conversation

joverlee521
Copy link
Contributor

Description of proposed changes

Consolidates the pathogen-repo-ci and the pathogen-repo-build by having pathogen-repo-ci call on pathogen-repo-build. Suggested by @tsibley in #62 (comment).

Technically the nested workflow is doable and it works (at least in this repo's CI job), but I'm not a big fan of the downsides of this approach.

There are two major downsides:

  1. We must hardcode the ref used for the pathogen-repo-build. We cannot
    get around this with the workflow-context action because this is calling
    a resuable workflow as a full job rather than an action step within a job.

  2. We have to define additional permissions from the caller for both
    the pathogen-repo-ci and the pathogen-repo-build with these updates
    because if you specify the access for any of the scopes, all of those
    that are not specified are set to none.

Checklist

  • Checks pass

This is done in prepartion for consolidating the pathogen-repo-ci and
pathogen-repo-build workflows by having the pathogen-repo-ci calling
the pathogen-repo-build.

Creating this commit first because the pathogen-repo-ci will need to
hardcode the sha for this version of the pathogen-repo-build.
This allows the pathogen-repo-ci to continue to manage the complexity
related to testing multiple runtimes, but surfaces the flexibility of
the pathogen-repo-build inputs.

There are two major downsides to this approach:
1. We must hardcode the ref used for the pathogen-repo-build. We cannot
get around this with the workflow-context action because this is calling
a resuable workflow as a full job rather than an action step within a job.
2. We have to define additional permissions from the caller for both
the pathogen-repo-ci and the pathogen-repo-build with these updates
because if you specify the access for any of the scopes, all of those
that are not specified are set to none.
Add the permissions for the pathogen-repo-ci and pathogen-repo-build
workflows that are now required due to the consolidation of the two
workflows.
@genehack
Copy link
Contributor

genehack commented May 9, 2024

Based on Jover's original description of the drawbacks with this approach and the lack of further movement in the ~6 months since, I think this is closable?

@joverlee521
Copy link
Contributor Author

Thanks for flagging @genehack!

This is probably not the direction we'll be taking with pathogen CIs, so closing!

@joverlee521 joverlee521 closed this May 9, 2024
@joverlee521 joverlee521 deleted the nested-pathogen-repo-ci branch May 9, 2024 20:46
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