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

🐛 Fix the Code review merged by user reference #1272

Closed
wants to merge 5 commits into from

Conversation

naveensrinivasan
Copy link
Member

  • Please check if the PR fulfills these requirements

@naveensrinivasan naveensrinivasan changed the title 🐛 Fix the Code review 🐛 Fix the Code review #1260 Nov 15, 2021
@github-actions
Copy link

Integration tests success for a92cbdff21f84aa283d0e31eee2908045e511304

@github-actions
Copy link

Integration tests success for 158cc51ae03a54697d731c69ee0945f32efc2416

@naveensrinivasan naveensrinivasan changed the title 🐛 Fix the Code review #1260 🐛 Fix the Code review merged by user reference Nov 15, 2021
@laurentsimon laurentsimon self-requested a review November 15, 2021 22:44
@laurentsimon
Copy link
Contributor

I think this is a draft. Let me know when you need a review.

@naveensrinivasan
Copy link
Member Author

I think this is a draft. Let me know when you need a review.

Why do you think this is a draft? What else is missing?

@github-actions
Copy link

Integration tests failure for 45ddaaf92528d2361f8a84db1f60b7535209ee23

@laurentsimon
Copy link
Contributor

I think this is a draft. Let me know when you need a review.

Why do you think this is a draft? What else is missing?

I don't see changes to the Code-Review check itself, so I'm assuming it's not aware of the new MergedBy value...

@laurentsimon
Copy link
Contributor

laurentsimon commented Nov 16, 2021

Before merging this PR, let's verify it does not break this earlier fix #895 for this issue #1055. (These corner cases should be added to the overall unit tests you're working on - not this PR)

@github-actions
Copy link

Stale pull request message

@azeemshaikh38
Copy link
Contributor

are we ready to submit this PR or is there anything pending here?

@laurentsimon
Copy link
Contributor

can we add unit tests to verify what the PR fixes? I think we're also missing some checks for previous PR #1272 (comment).
I'm just afraid to break other changes we've made... since we don't have unit tests for it. If we break, we go in circles :-)
wdut?

@azeemshaikh38
Copy link
Contributor

can we add unit tests to verify what the PR fixes? I think we're also missing some checks for previous PR #1272 (comment). I'm just afraid to break other changes we've made... since we don't have unit tests for it. If we break, we go in circles :-) wdut?

+1 to adding unit tests. No rush on submitting this PR. Just trying to reduce the number of open issues so wanted to check if this PR can close the linked issue.

@naveensrinivasan
Copy link
Member Author

Agreed on the unit tests!

@github-actions
Copy link

Stale pull request message

@github-actions github-actions bot closed this Jan 16, 2022
@laurentsimon laurentsimon reopened this Jan 18, 2022
@laurentsimon laurentsimon temporarily deployed to integration-test January 18, 2022 16:28 Inactive
@github-actions
Copy link

Integration tests success for
[0621c8c]
(https://github.com/ossf/scorecard/actions/runs/1713771872)

@github-actions github-actions bot closed this Feb 8, 2022
@laurentsimon laurentsimon deleted the fix/code-review-author branch February 14, 2023 22:21
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