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

Update Doc for aws propagator default sampling behavior #658

Closed
wants to merge 3 commits into from

Conversation

willarmiros
Copy link

Description

Dupe of #656, for also testing CODEOWNERS

willarmiros and others added 2 commits September 2, 2021 13:51
Co-authored-by: Nathaniel Ruiz Nowell <enowell@amazon.com>
@willarmiros willarmiros requested a review from a team as a code owner September 2, 2021 21:58
@owais owais added the Skip Changelog PRs that do not require a CHANGELOG.md entry label Sep 2, 2021
@NathanielRN
Copy link
Contributor

Hm interesting, maybe because I am part of the commit history I didn't get set a reviewer even though I am a CODEOWNER? But I can still give an approving review?

@owais
Copy link
Contributor

owais commented Sep 2, 2021

May be. I think we should update the check so that it is only required when the author is not a code-owner.

@NathanielRN
Copy link
Contributor

Also it looks like the "Component Owners" test is failing 😕 @lzchen

Run dyladan/component-owners@main
  with:
    assign-owners: true
    request-owner-reviews: true
    repo-token: ***
    config-file: .github/component_owners.yml
1 owners found NathanielRN
Adding assignees
Error: Resource not accessible by integration

@willarmiros
Copy link
Author

Yeah sorry, totally didn't think of that when giving you the co-author @NathanielRN

@NathanielRN
Copy link
Contributor

No worries at all @willarmiros ! I think it's useful to see what this action does. We know the CODEOWNER works and the test passes from #657, but we should figure out the change in this PR.

@lzchen had a good idea that this is because it's from your fork wheras #657 was from an upstream branch. I remember seeing the Error: Resource not accessible by integration a lot such as in this GitHub Issue when doing operations on the repo.

@NathanielRN
Copy link
Contributor

Closing in favor of #656 now that we checked the CODEOWNERs tool is working as expected.

@NathanielRN NathanielRN closed this Sep 8, 2021
@codeboten
Copy link
Contributor

Closing in favor of #656 now that we checked the CODEOWNERs tool is working as expected.

I... this was really confusing. The file diff made it look like this was being added in a different place lol

@NathanielRN
Copy link
Contributor

I... this was really confusing. The file diff made it look like this was being added in a different place lol

Ah really sorry about that! Testing CODEOWNERs got in the middle of me deciding the change would fit better at a different point in the docs 😓 I really appreciate your green check mark! Especially since it helped me realize I should've closed this one immediately once #656 got merged but better late than never I guess 😅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Skip Changelog PRs that do not require a CHANGELOG.md entry
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants