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

Multi Provider #1

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

Multi Provider #1

wants to merge 17 commits into from

Conversation

ajwootto
Copy link
Collaborator

@ajwootto ajwootto commented May 2, 2024

  • WIP draft of vendor migration / meta / multi / omega provider based on the original PR

@ajwootto ajwootto changed the title Ajwootto/meta provider Multi Provider May 22, 2024
};

export class MultiProvider implements Provider {
readonly runsOn = 'server';
Copy link
Member

Choose a reason for hiding this comment

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

I guess this brings up the question (probably fo the OF folks), should this be named the ServerMultiProvider?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

that wouldn't match the other provider naming conventions used in this repo, where an "unqualified" name seems to be assumed to mean "server", and web providers add "web" to the name

provider: Provider;
};

export class MultiProvider implements Provider {
Copy link
Member

Choose a reason for hiding this comment

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

More general thoughts:

  • Might need some object types to encapsulate passing around so many params between methods.
  • Can or should the handler / status / hooks code be moved to other classes?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

not really sure about the status thing, it's pretty intertwined with the rest of what's in here so it might be kind of an unnecessary indirection to move it

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

actually i tried moving it and it wasnt that bad

@ajwootto ajwootto force-pushed the ajwootto/meta-provider branch 5 times, most recently from 667c018 to bb17e3f Compare May 23, 2024 19:06
toddbaert and others added 2 commits May 23, 2024 15:38
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@ajwootto ajwootto force-pushed the ajwootto/meta-provider branch 5 times, most recently from 2a8f2df to 30245ad Compare May 23, 2024 21:17
renovate bot added 6 commits May 28, 2024 11:43
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
…#920)

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
)

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
)

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
@ajwootto ajwootto force-pushed the ajwootto/meta-provider branch 7 times, most recently from 17d23f7 to 271ca0e Compare June 6, 2024 14:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants