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

Move internal wiki page "Subscribe to Notifications" next to tools/notification-configuration #4837

Merged
merged 2 commits into from
Dec 2, 2022

Conversation

konrad-jamrozik
Copy link
Contributor

@konrad-jamrozik konrad-jamrozik commented Nov 30, 2022

This PR adds a README.md file to tools/notification-configuration.

This README most notably just points to the wiki article describing the feature that is implemented by the sources in the directory to which the README was added, i.e.:
https://dev.azure.com/azure-sdk/internal/_wiki/wikis/internal.wiki/63/Subscribe-to-Notifications

As accompanying change to this PR, I made several updates to the doc hyperlinked from the README. You can find the full diff of my wiki changes here.

image

@konrad-jamrozik konrad-jamrozik requested a review from a team as a code owner November 30, 2022 08:35
@konrad-jamrozik
Copy link
Contributor Author

konrad-jamrozik commented Nov 30, 2022

@weshaggard @hallipr I have few questions:

Q1. How is this tool being used?
https://github.com/Azure/azure-sdk-tools/blob/main/tools/notification-configuration/github-codeowner-subscriber/Program.cs

I see there is a package for it:
https://dev.azure.com/azure-sdk/internal/_artifacts/feed/azure-sdk-for-net/NuGet/Azure.Sdk.Tools.GithubCodeownerSubscriber/1.0.0-dev.20221129.1

but I didn't find any other mentions of it. Is this package still relevant? How is it supposed to be used?

Q2. In the README I added, I added a reference to our internal wiki, which I am not sure makes sense, given this is a public repo. But then again, if the tool is public, shouldn't the doc for it also be public?

@weshaggard
Copy link
Member

Q1: It is used in https://github.com/Azure/azure-sdk-tools/blob/main/eng/pipelines/notifications.yml which is a pipeline we run daily at https://dev.azure.com/azure-sdk/internal/_build?definitionId=679

Q2: We try to avoid linking to our internal wiki from public repos (link from our internal wiki to public repo where needed), but there are some exceptions to that rule. As a general rule try to document everything in the open repo and we only keep things internal if there is something sensitive. So for this tool I suspect we can move things out here.

@konrad-jamrozik
Copy link
Contributor Author

konrad-jamrozik commented Nov 30, 2022

Q1: It is used in https://github.com/Azure/azure-sdk-tools/blob/main/eng/pipelines/notifications.yml which is a pipeline we run daily at https://dev.azure.com/azure-sdk/internal/_build?definitionId=679

@weshaggard I am sorry if I am being dense, but I see only usages of notification-creator (aka notification-configurator) but not of github-codeowner-subscriber (Azure.Sdk.Tools.GithubCodeownerSubscriber). Basically I cannot find any usages of this method: Azure.Sdk.Tools.GithubCodeownerSubscriber.Program.Main.

Can you point me to the line in notifications.yml that uses it? I believe the call to noification-creator executes Azure.Sdk.Tools.NotificationConfiguration.Program.Main and not Azure.Sdk.Tools.GithubCodeownerSubscriber.Program.Main.

@weshaggard
Copy link
Member

weshaggard commented Nov 30, 2022

Basically I cannot find any usages of this method: Azure.Sdk.Tools.GithubCodeownerSubscriber.Program.Main.

Sorry I think I misunderstood your question. It is possible that might not be used any longer. @sima-zhu do you know if we use that tool for anything anymore?

@sima-zhu
Copy link
Contributor

sima-zhu commented Dec 1, 2022

Basically I cannot find any usages of this method: Azure.Sdk.Tools.GithubCodeownerSubscriber.Program.Main.

Sorry I think I misunderstood your question. It is possible that might not be used any longer. @sima-zhu do you know if we use that tool for anything anymore?

We merged this step together with notification creator, so it is no longer in use. Thanks @konrad-jamrozik for bringing this up! Created a work item for the cleanup.

@konrad-jamrozik konrad-jamrozik added the Central-EngSys This issue is owned by the Engineering System team. label Dec 2, 2022
@konrad-jamrozik konrad-jamrozik self-assigned this Dec 2, 2022
@konrad-jamrozik konrad-jamrozik changed the title Add a README.md to tools/notification-configuration + update "Subscribe to Notifications" wiki page Move internal wiki page "Subscribe to Notifications" next to tools/notification-configuration Dec 2, 2022
@konrad-jamrozik konrad-jamrozik changed the title Move internal wiki page "Subscribe to Notifications" next to tools/notification-configuration Move internal wiki page "Subscribe to Notifications" next to tools/notification-configuration Dec 2, 2022
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.

Feel free to gradually move the content out to the README.md

@konrad-jamrozik konrad-jamrozik merged commit edcaffd into main Dec 2, 2022
@konrad-jamrozik konrad-jamrozik deleted the users/kojamroz/notif-readme branch December 2, 2022 20:38
@konrad-jamrozik konrad-jamrozik restored the users/kojamroz/notif-readme branch December 2, 2022 21:44
@konrad-jamrozik konrad-jamrozik deleted the users/kojamroz/notif-readme branch December 5, 2022 22:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Central-EngSys This issue is owned by the Engineering System team.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants