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

Transport support for Skpr #66

Merged
merged 2 commits into from
Dec 1, 2022

Conversation

fubarhouse
Copy link
Contributor

Overview

This pull request:

Q A
Bug fix? no
New feature? no
Has tests? yes
BC breaks? no
Deprecations? no

Summary

Enables support for Drush commands to wrap skpr commands, enabling users on the Skpr platform to be able to use Drush and/or Drush focused scripts for management, migration, debugging etc.

Description

I really loved that this could enable me to interface with Drupal via kubectl commands, we saw this as an opportunity to do the same with our skpr command line tool - because developers using our platform won't have access to kubectl for their day-to-day work.

I've modeled this work based on the kubectl transport work, and it's working quite nicely.

Signed-off-by: Karl Hepworth <[email protected]>
Signed-off-by: Karl Hepworth <[email protected]>
@codecov
Copy link

codecov bot commented Aug 25, 2022

Codecov Report

Merging #66 (9c32421) into main (e1b77de) will decrease coverage by 1.24%.
The diff coverage is 52.38%.

@@             Coverage Diff              @@
##               main      #66      +/-   ##
============================================
- Coverage     78.46%   77.22%   -1.25%     
- Complexity      165      172       +7     
============================================
  Files            18       20       +2     
  Lines           418      439      +21     
============================================
+ Hits            328      339      +11     
- Misses           90      100      +10     
Impacted Files Coverage Δ
src/Factory/SkprTransportFactory.php 50.00% <50.00%> (ø)
src/Transport/SkprTransport.php 50.00% <50.00%> (ø)
src/ProcessManager.php 77.50% <100.00%> (+0.57%) ⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@nickschuch
Copy link

Might be better to contribute to this #61 and then provide docs on the platforms site.

@greg-1-anderson
Copy link
Member

Maybe everything should have been done like #61, but I am left with questions on that approach. It seems safer to make each transport explicit, so that each one can define the parameters it needs, do any special processing that is appropriate, and etc.

If nothing else, it's easier to make a decision about a single well-defined transport. That's why I am merging here, and am still undecided about #61.

@greg-1-anderson greg-1-anderson merged commit 49ec2b9 into consolidation:main Dec 1, 2022
@jonhattan jonhattan mentioned this pull request Mar 11, 2024
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.

3 participants