-
Notifications
You must be signed in to change notification settings - Fork 14
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
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this 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 I was trying to do something similar as you mentioned. My first idea was to include that option ( |
Here's the relevant bit of code that will need modifying https://github.com/Lullabot/drainpipe/blob/main/src/ScaffoldInstallerPlugin.php#L285 |
ddd39f7
to
f57c15e
Compare
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. |
8a73b3d
to
c251e46
Compare
We are running this branch on one customer site. I'd like to get reviews from folks before copying it to 10 other repos. |
@leonel-lullabot Is there anything here you wanted to update or notes you wanted to leave for reviewers? |
@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. |
There was a problem hiding this 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
@leonel-lullabot could we get latest tag merged here? |
@elvism-lullabot Done 👍 |
@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. |
0d34004
to
722a25e
Compare
@YesCT Thanks for explaining why this pull request was not ready to be rebased with We will want to update this branch with the latest in |
I'm not quite following what went on there 😄 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.) |
@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 |
Yeah, next time I'll bring in unmerged code differently. I made the other branch to continue development. #670 |
This branch is in use on customer sites.
Please develop using #670 . Sorry for the troubles.
Relates to:
Description:
pantheon-multidev
is added