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

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

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 had a problem deploying to pantheon-pr-533 April 16, 2024 19:52 Failure
@justafish
Copy link
Member

[nesting intensifies]

@github-actions github-actions bot had a problem deploying to pantheon-pr-533 April 17, 2024 11:29 Failure
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 👍

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

3 participants