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

Initial KeyVaultProxy solution #15123

Merged
merged 2 commits into from
Sep 25, 2020
Merged

Conversation

heaths
Copy link
Member

@heaths heaths commented Sep 12, 2020

Ported with permission from @heaths from https://github.com/heaths/KeyVaultProxy. Builds both within and without the Azure/azure-sdk-for-net repo.

@heaths heaths requested a review from schaabs as a code owner September 12, 2020 12:02
@ghost ghost added the KeyVault label Sep 12, 2020
@heaths
Copy link
Member Author

heaths commented Sep 12, 2020

I'm moving this from my person repo where I used it as an example for both the KV service team and on social like Stack Overflow. It builds in and out of the repo correctly.

I am considering, though, that maybe the README with the sample front matter should be only in the src directory, since:

  1. The tests really aren't needed to be part of the sample, right?
  2. I can more easily consume the existing test framework we have already for live tests - whether we run them nightly or not - rather than duplicate a few TestFramework sources and use my xunit-based test discovery.

Do people agree I should just make the src project itself the published sample and leave the tests here? Should I make sure they run as part of nightlies?

Copy link
Member

@weshaggard weshaggard left a comment

Choose a reason for hiding this comment

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

I didn't look deeply at the code but the general structure looks fine. I assume you add the snippets to make sure they stay updated in the MD.

@weshaggard
Copy link
Member

Do people agree I should just make the src project itself the published sample and leave the tests here? Should I make sure they run as part of nightlies?

I'm Ok with having the tests here, but I'm not sure we should add them to our nightly test runs. Eventually we should get the sample code running in our smoke tests and given this is kind of a sample library we should either write a sample application or maybe use the tests as a proxy and run as part of those smoke tests.

@heaths
Copy link
Member Author

heaths commented Sep 24, 2020

I'm Ok with having the tests here, but I'm not sure we should add them to our nightly test runs.

I agree keeping the test source is good, but I was referring more to whether only src should be published as the sample (i.e. the sample README should go there). The more I think about it, the more I think that's probably a better call. The tests and structure to support the tests only unnecessarily complicate the sample.

As for getting them running, I'll open a test issue on the backlog. I had considered this, but even getting sample source that builds and runs correctly in our repo and as a standalone downloaded sample is enough of a pain without complicating the source. I brought this up yesterday in our .NET team meeting as well (other people are looking to add samples) and want to with you more about it.

heaths and others added 2 commits September 25, 2020 01:28
Ported with permission from @heaths from https://github.com/heaths/KeyVaultProxy. Builds both within and without the Azure/azure-sdk-for-net repo.
@heaths heaths force-pushed the keyvaultproxy-sample branch from d97f9ff to 5aa354a Compare September 25, 2020 08:39
@heaths heaths merged commit a1bda01 into Azure:master Sep 25, 2020
@heaths heaths deleted the keyvaultproxy-sample branch September 25, 2020 08:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants