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

fix: adjust pipelines by using submodules #3382

Merged
merged 9 commits into from
May 12, 2023

Conversation

LetItRock
Copy link
Contributor

What change does this PR introduce?

Adjust the GitHub pipelines to also use the submodules. In this PR:

  • clone the submodules only for the enterprise builds
  • clone the submodules only for the e2e tests for: web, widget, API, worker
  • moved the checkout action to project setup

Why was this change needed?

Other information (Screenshots)

@LetItRock LetItRock self-assigned this May 10, 2023
@linear
Copy link

linear bot commented May 10, 2023

NV-2288 Adjust the pipelines to clone submodules

Why? (Context)

What?

Definition of Done

Copy link
Contributor

@scopsy scopsy left a comment

Choose a reason for hiding this comment

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

Overall looks great! For some reason seems like some of the previous actions are failing tho

@@ -25,6 +25,7 @@ jobs:
uses: ./.github/workflows/reusable-api-e2e.yml
with:
ee: ${{ contains (matrix.name,'ee') }}
submodules: true
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't we add this here:

Suggested change
submodules: true
submodules: ${{ contains (matrix.name,'ee') }}

?

Copy link
Contributor Author

@LetItRock LetItRock May 10, 2023

Choose a reason for hiding this comment

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

@scopsy the above workflow reusable-api-e2e.yml is running unit and e2e tests that's why I thought that this submodules flag should be always true
cc @Cliftonz

@@ -14,6 +14,7 @@ jobs:
uses: ./.github/workflows/reusable-api-e2e.yml
with:
ee: ${{ contains (matrix.name,'ee') }}
submodules: true
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
submodules: true
submodules: ${{ contains (matrix.name,'ee') }}

@@ -14,6 +14,7 @@ jobs:
uses: ./.github/workflows/reusable-worker-e2e.yml
with:
ee: ${{ contains (matrix.name,'ee') }}
submodules: true
Copy link
Contributor

Choose a reason for hiding this comment

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

same as above

@LetItRock LetItRock force-pushed the nv-2288-submodules-adjust-pipelines branch 2 times, most recently from dbff1a9 to efaa9d2 Compare May 11, 2023 12:28
@LetItRock LetItRock force-pushed the nv-2288-submodules-adjust-pipelines branch from efaa9d2 to c93dac3 Compare May 11, 2023 12:31
@LetItRock LetItRock force-pushed the nv-2288-submodules-adjust-pipelines branch 7 times, most recently from 0a94bdd to 84a9010 Compare May 11, 2023 15:29
@LetItRock LetItRock force-pushed the nv-2288-submodules-adjust-pipelines branch from 84a9010 to c62efe0 Compare May 11, 2023 15:31
@davidsoderberg davidsoderberg merged commit 9b05dc1 into timed-digest May 12, 2023
15 of 16 checks passed
@davidsoderberg davidsoderberg deleted the nv-2288-submodules-adjust-pipelines branch May 12, 2023 06:59
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.

None yet

4 participants