Skip to content

Update codeowners#15869

Merged
AlexGhiondea merged 20 commits intoAzure:masterfrom
AlexGhiondea:UpdateCodeowners
Nov 3, 2020
Merged

Update codeowners#15869
AlexGhiondea merged 20 commits intoAzure:masterfrom
AlexGhiondea:UpdateCodeowners

Conversation

@AlexGhiondea
Copy link
Copy Markdown
Contributor

No description provided.

Comment thread .github/CODEOWNERS Outdated
Comment thread .github/CODEOWNERS Outdated
Comment thread .github/CODEOWNERS Outdated
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Azure.Core is not the right one for this one.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Ah okay my bad - we can leave it as NotInRepo instead then

Comment thread .github/CODEOWNERS Outdated
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This one might just be for the resource manager for Cosmos... not for all of Cosmos... We should ask

Comment thread .github/CODEOWNERS Outdated
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This folder only contains the resource manager for network... That might be the right thing -- but not 100% sure.

Comment thread .github/CODEOWNERS Outdated
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'd consider removing %Service Attention duplication in this file and have the tool that sets up the rule add this label automatically.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Talked about this offline -- we'll leave this in for now.

Copy link
Copy Markdown
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.

Made one comment but otherwise looks good.

AlexGhiondea and others added 9 commits October 20, 2020 10:58
@AlexGhiondea AlexGhiondea marked this pull request as ready for review October 20, 2020 18:00
Add the Event Hubs, Event Grid and Service Bus owners.
Update Event Hubs CODEOWNER
@jsquire
Copy link
Copy Markdown
Member

jsquire commented Oct 23, 2020

@AlexGhiondea: You may want to consider pulling in the additions from #16200 and #16189

Comment thread .github/CODEOWNERS

# ######## Management Plane ########

/**/*Management*/ @allenjzhang @m-nash @markcowl @YalinLi0312 @bquantump @nisha-bhatia
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This one is broad and catches things like changes to sdk/servicebus/Azure.Messaging.ServiceBus/tests/Administration/ServiceBusManagementClientLiveTests.cs. I don't have a good suggestion for tuning it, but maybe you do?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I will leave that as-is for now... this particular one is only used for the PR owner suggestions.

Comment thread .github/CODEOWNERS
Comment thread .github/CODEOWNERS Outdated
Comment thread .github/CODEOWNERS
Comment thread .github/CODEOWNERS
Comment thread .github/CODEOWNERS Outdated
Comment thread .github/CODEOWNERS Outdated
@AlexGhiondea
Copy link
Copy Markdown
Contributor Author

/check-enforcer override

@jsquire
Copy link
Copy Markdown
Member

jsquire commented Oct 30, 2020

@AlexGhiondea: You'll likely want to absorb the additions from ##16440 as well.

@AlexGhiondea
Copy link
Copy Markdown
Contributor Author

/check-enforcer override

@check-enforcer
Copy link
Copy Markdown

check-enforcer Bot commented Nov 3, 2020

This pull request is protected by Check Enforcer.

What is Check Enforcer?

Check Enforcer helps ensure all pull requests are covered by at least one check-run (typically an Azure Pipeline). When all check-runs associated with this pull request pass then Check Enforcer itself will pass.

Why am I getting this message?

You are getting this message because Check Enforcer did not detect any check-runs being associated with this pull request within five minutes. This may indicate that your pull request is not covered by any pipelines and so Check Enforcer is correctly blocking the pull request being merged.

What should I do now?

If the check-enforcer check-run is not passing and all other check-runs associated with this PR are passing (excluding license-cla) then you could try telling Check Enforcer to evaluate your pull request again. You can do this by adding a comment to this pull request as follows:
/check-enforcer evaluate
Typically evaulation only takes a few seconds. If you know that your pull request is not covered by a pipeline and this is expected you can override Check Enforcer using the following command:
/check-enforcer override
Note that using the override command triggers alerts so that follow-up investigations can occur (PRs still need to be approved as normal).

What if I am onboarding a new service?

Often, new services do not have validation pipelines associated with them, in order to bootstrap pipelines for a new service, you can issue the following command as a pull request comment:
/azp run prepare-pipelines
This will run a pipeline that analyzes the source tree and creates the pipelines necessary to build and validate your pull request. Once the pipeline has been created you can trigger the pipeline using the following comment:
/azp run net - [service] - ci

Ensure that for WebJobs we label PRs.
Clarify the storage section.
Comment thread .github/CODEOWNERS
# PRLabel: %DigitalTwins
/sdk/digitaltwins/ @drwill-ms @timtay-microsoft @abhipsaMisra @vinagesh @azabbasi @bikamani @barustum @jamdavi
# ServiceLabel: %Digital Twins %Service Attention
/sdk/digitaltwins/ @drwill-ms @timtay-microsoft @abhipsaMisra @vinagesh @azabbasi @bikamani @barustum @sourabhguha @inesk-vt @jamdavi
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why did @sourabhguha and @inesk-vt get added? I'm not against it, but noticing this is a change. They may not want all the PR email.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This comes from the internal wiki list. I will add them there for now and if there is too much traffic we can remove.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I checked with Saurabh, and he asked me to remove him. I'm checking with Ines.

Is there any benefit to them to be on this list, aside from PR participation?

Comment thread .github/CODEOWNERS Outdated
Remove person not on the team anymore.
@AlexGhiondea
Copy link
Copy Markdown
Contributor Author

/check-enforcer override

@AlexGhiondea AlexGhiondea merged commit 834641b into Azure:master Nov 3, 2020
annelo-msft pushed a commit to annelo-msft/azure-sdk-for-net that referenced this pull request Feb 17, 2021
* Add codeowners to the file

* Add more changes to the codeowners file.

* Re-arrange codeowners file

* Move the Management and EngSys sections to the top of the file.

* Remove angled brackets from the file to not make it look like we have merge conflicts.

* Update CODEOWNERS

added some more folders to service labels I found

* Update CODEOWNERS

Update 2 owners.

* Update CODEOWNERS

Update the ARM - Core entry to not point to Azure.Core

* Remove the extra comment around the actual entries

Doing this so that we reduce the amount of comments in the file which could lead to confusion.

* Update CODEOWNERS

Add the Event Hubs, Event Grid and Service Bus owners.

* Update CODEOWNERS

Update Event Hubs CODEOWNER

* Update CODEOWNERS

Update EventGrid.

* Update CODEOWNERS

Fix issue with the PR Label.

* Update CODEOWNERS

Incorporate Azure#16200

* Update CODEOWNERS

Incorporate Azure#16363

* Update CODEOWNERS

Incorporate Azure#16440

* Update CODEOWNERS

Ensure that for WebJobs we label PRs.

* Update CODEOWNERS

Clarify the storage section.

* Update CODEOWNERS

Remove person not on the team anymore.

Co-authored-by: Meera Haridasa <22649188+meeraharidasa@users.noreply.github.com>
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.

8 participants