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

[In Use: update with release] Issue #535: Change multidev deploy to manual only: tag based #546

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

Conversation

leonel-lullabot
Copy link
Collaborator

@leonel-lullabot leonel-lullabot commented Apr 24, 2024

This branch is in use on customer sites.

Please develop using #670 . Sorry for the troubles.

Relates to:

Description:

  • Run the Multidev Deploy when the label pantheon-multidev is added

Copy link
Contributor

@YesCT YesCT left a comment

Choose a reason for hiding this comment

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

I'm not sure how to make this configurable. But this PR is good for demonstrating the alternative.


Maybe

"extra": {
    "drainpipe": {
        "github": ["PantheonReviewApps"]
    }
}

could be the default and

"extra": {
    "drainpipe": {
        "github": ["PantheonReviewAppsManual"]
    }
}

could switch to this manual workflow_dispatch ??

I'm not sure what config could look like to send an option to PantheonReviewApps.


Keeping this as Draft is good to indicate this is not ready for merge.

@YesCT YesCT requested a review from davereid April 25, 2024 17:57
@leonel-lullabot
Copy link
Collaborator Author

@YesCT I was trying to do something similar as you mentioned. My first idea was to include that option (PantheonReviewAppsManual) and clone the two worflows (PantheonReviewApps/PantheonReviewAppsDDEV) but using workflow_dispatch. This should work, but I wasn't sure it was a good idea to clone those workflows.

@justafish
Copy link
Member

Here's the relevant bit of code that will need modifying https://github.com/Lullabot/drainpipe/blob/main/src/ScaffoldInstallerPlugin.php#L285

@YesCT
Copy link
Contributor

YesCT commented May 2, 2024

Ran into an error with the multidev pr number being a string ("renovate") when trying this out on a customer project. There will probs be some more commits here before this is ready to move out of Draft.

@github-actions github-actions bot requested a deployment to pantheon-pr-546 May 2, 2024 20:34 In progress
@leonel-lullabot leonel-lullabot force-pushed the 535--change-multidev-deploy-to-manual-only branch from 8a73b3d to c251e46 Compare May 2, 2024 20:37
@YesCT YesCT changed the title Issue #535: Change multidev deploy to manual only Issue #535: Change multidev deploy to manual only: tag based May 8, 2024
@YesCT
Copy link
Contributor

YesCT commented May 8, 2024

We are running this branch on one customer site. I'd like to get reviews from folks before copying it to 10 other repos.

@YesCT
Copy link
Contributor

YesCT commented May 8, 2024

@leonel-lullabot Is there anything here you wanted to update or notes you wanted to leave for reviewers?

@leonel-lullabot
Copy link
Collaborator Author

@YesCT Recently, I noticed that when a Pull Request is updated, the Multidev deploy doesn't happen unless we remove the label and add it again, so I think it would be good to improve this process.

@leonel-lullabot leonel-lullabot marked this pull request as ready for review May 21, 2024 14:47
@YesCT YesCT requested a review from justafish May 21, 2024 16:20
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.

Instead of duplicating these files could we

@elvism-lullabot
Copy link
Collaborator

@leonel-lullabot could we get latest tag merged here?

@leonel-lullabot
Copy link
Collaborator Author

@elvism-lullabot Done 👍

@github-actions github-actions bot had a problem deploying to pantheon-pr-546 August 26, 2024 23:02 Failure
@YesCT
Copy link
Contributor

YesCT commented Aug 27, 2024

@mrdavidburns We don't want to use main on this branch. We want to use the release. Otherwise, we are running ahead of the release on the customer site.

(And drainpipe won't match with drainpipe-dev, though I'm not clear on when that matters. @justafish or @davereid might be able to explain when it matters.)

Maybe we should have one branch that is uptodate with main, for possible merging and review and testing,

and one that is uptodate with the release, for us to use on the customer site.

@mrdavidburns mrdavidburns force-pushed the 535--change-multidev-deploy-to-manual-only branch from 0d34004 to 722a25e Compare August 27, 2024 12:21
@mrdavidburns
Copy link
Member

@YesCT Thanks for explaining why this pull request was not ready to be rebased with main. I went ahead and removed that commit from this branch.

We will want to update this branch with the latest in main and confirm all checks passed before this is merged.

@justafish
Copy link
Member

I'm not quite following what went on there 😄
drainpipe-dev branches now get updated alongside branches opened in this repository, but there's no drainpipe-dev changes here

This branch needs updating with main and the changes requested addressed please 🙏

@YesCT YesCT changed the title Issue #535: Change multidev deploy to manual only: tag based [In Use: update with release] Issue #535: Change multidev deploy to manual only: tag based Aug 28, 2024
@github-actions github-actions bot requested a deployment to pantheon-pr-670 August 28, 2024 13:38 In progress
@YesCT
Copy link
Contributor

YesCT commented Aug 28, 2024

I'm not quite following what went on there 😄 drainpipe-dev branches now get updated alongside branches opened in this repository, but there's no drainpipe-dev changes here

This branch needs updating with main and the changes requested addressed please 🙏

I moved the feedback to #670

Please let's use that PR for development. (Sorry but I'm afraid to develop on this branch, while I'm using it on several customer repos.) I think we can avoid this in the future, by testing on a test site (lsm-examples), and not live customer sites. Part of my fear, is that when this branch were to be updated with main, our customer repos would still be on an old version of drainpipe-dev, and I'm not sure what troubles having those out of sync could cause. Part of my fear, is more simple, that we'd try pushing something here, that might temporarily interrupt our working build and the customer might be impacted.)

@justafish
Copy link
Member

@YesCT Whilst you could pin to a specific commit, I recommend you move your customer sites to using their own custom templates for this which aren't attached to Drainpipe, or your own fork - we can't really block active development on this branch and once it's merged it'll go away immediately

@YesCT
Copy link
Contributor

YesCT commented Aug 28, 2024

Yeah, next time I'll bring in unmerged code differently.

I made the other branch to continue development. #670

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.

5 participants