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

auth: Refactor AzureDevOpsSubscriptionProvider so that it accepts values as arguments #1729

Merged
merged 2 commits into from
May 15, 2024

Conversation

hossam-nasr
Copy link
Contributor

[auth package] modify the AzureDevOpsSubscriptionProvider and AzureDevOpsSubscriptionProviderFactory so that the three values required to identify the service connection: service connection ID, domain, and client ID, are passed as arguments to the constructor, rather than defaulting to being set as environment variables in the way we currently do. This allows for more versatility and flexibility in the way this provider can be used by other parties and users wanting to do their own E2E testing

@hossam-nasr hossam-nasr requested a review from a team as a code owner May 14, 2024 19:14
@alexweininger
Copy link
Member

alexweininger commented May 14, 2024

The most nitpick I've ever done: Can you prefix the PRs like "auth:" instead of "[auth]". We've been doing it that way and I want to stay consistent (as much as possible).

The reasoning is when we generate changelogs on github it's much easier when all the commits are in the same format.

@hossam-nasr hossam-nasr changed the title [auth] Refactor AzureDevOpsSubscriptionProvider so that it accepts values as arguments auth: Refactor AzureDevOpsSubscriptionProvider so that it accepts values as arguments May 14, 2024
@hossam-nasr
Copy link
Contributor Author

@alexweininger lol yes done 🫡

nturinski
nturinski previously approved these changes May 14, 2024
Copy link
Member

@nturinski nturinski left a comment

Choose a reason for hiding this comment

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

LGTM 🫡

auth/package.json Outdated Show resolved Hide resolved
@hossam-nasr hossam-nasr merged commit b0db00a into main May 15, 2024
4 checks passed
@hossam-nasr hossam-nasr deleted the hossamnasr/auth-refactor branch May 15, 2024 00:06
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