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

Pull gitlab / github installer code into new methods #533

Merged
merged 10 commits into from
Jul 19, 2024

Conversation

deviantintegral
Copy link
Member

This PR pulls is the first two commits from https://github.com/Lullabot/drainpipe/pull/114/commits that:

  1. Extracts private methods to reduce the length of the installCiCommands method.
  2. Adds an early return to manage some of the intense nesting the code currently has.

@github-actions github-actions bot temporarily deployed to pantheon-pr-533 April 16, 2024 19:52 Destroyed
@justafish
Copy link
Member

[nesting intensifies]

@github-actions github-actions bot temporarily deployed to pantheon-pr-533 April 17, 2024 09:56 Destroyed
@github-actions github-actions bot temporarily deployed to pantheon-pr-533 April 17, 2024 10:52 Destroyed
@github-actions github-actions bot temporarily deployed to pantheon-pr-533 April 17, 2024 11:29 Destroyed
Copy link
Member

@justafish justafish left a comment

Choose a reason for hiding this comment

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

@deviantintegral I fixed the database version error mismatch, which I thought was a result of a DDEV upgrade. I think the test failures are now genuine however as the Tugboat files don't seem to be be installed correctly.

@@ -2,6 +2,6 @@ api_version: 1
web_docroot: true
php_version: 8.0
database:
version: 10.4
version: 10.11
Copy link
Member

Choose a reason for hiding this comment

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

How do we feel about this change going out to all sites? Does this upgrade to several versions work okay on Pantheon deployments?

Copy link
Member

@davereid davereid Apr 25, 2024

Choose a reason for hiding this comment

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

I don't see 10.11 listed on https://docs.pantheon.io/pantheon-yml#specify-a-version-of-mariadb so does this need to be 10.6?

Copy link
Member

Choose a reason for hiding this comment

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

yeah good point, let's change this to 10.6 - currently we only have Pantheon integration for GitLab, but we'd like to have it for GitHub 👍

@github-actions github-actions bot temporarily deployed to pantheon-pr-533 May 1, 2024 11:46 Destroyed
@github-actions github-actions bot temporarily deployed to pantheon-pr-533 July 18, 2024 19:29 Destroyed
@github-actions github-actions bot temporarily deployed to pantheon-pr-533 July 19, 2024 12:10 Destroyed
@github-actions github-actions bot temporarily deployed to pantheon-pr-533 July 19, 2024 12:15 Destroyed
Copy link
Member

@mrdavidburns mrdavidburns left a comment

Choose a reason for hiding this comment

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

LGTM! Nice refactor.

@deviantintegral deviantintegral dismissed justafish’s stale review July 19, 2024 15:00

Database changes were fixed in other PRs

@deviantintegral deviantintegral merged commit 97dc322 into main Jul 19, 2024
36 checks passed
@deviantintegral deviantintegral deleted the gitlab-github-new-methods branch July 19, 2024 15:00
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