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.

Sign up for GitHub

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

Jump to bottom

QUIC: Fix incompatible merges causing CI breakage (urgent) #21641

Closed
wants to merge 1 commit into from
Closed

QUIC: Fix incompatible merges causing CI breakage (urgent) #21641

wants to merge 1 commit into from

Conversation

hlandau
Copy link
Member

@hlandau hlandau commented Aug 3, 2023

Fix CI breakage in master caused by interaction of two different PRs. Urgent due to CI breakage.

@hlandau hlandau added branch: master Merge to master branch approval: review pending This pull request needs review by a committer approval: otc review pending This pull request needs review by an OTC member triaged: bug The issue/pr is/fixes a bug severity: urgent Fixes an urgent issue (exempt from 24h grace period) tests: exempted The PR is exempt from requirements for testing labels Aug 3, 2023
@hlandau hlandau requested review from t8m, mattcaswell, paulidale and a team August 3, 2023 08:19
@hlandau hlandau self-assigned this Aug 3, 2023
@tom-cosgrove-arm tom-cosgrove-arm added approval: review pending This pull request needs review by a committer and removed approval: review pending This pull request needs review by a committer labels Aug 3, 2023
tom-cosgrove-arm
tom-cosgrove-arm requested changes Aug 3, 2023
View reviewed changes
Copy link
Contributor

@tom-cosgrove-arm tom-cosgrove-arm left a comment

Choose a reason for hiding this comment

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

CI looks relevant, but I don't understand - there are 9 arguments now

test/quicapitest.c:813:57: error: too few arguments to function call, expected 9, have 8
                                                    NULL))

@hlandau
Copy link
Member Author

hlandau commented Aug 3, 2023

@tom-cosgrove-arm I think the CIFuzz run must be testing against master rather than this PR. Possibly this was an oversight and it hasn't been testing PRs properly. We should fix that at some point if so. In any case, doesn't seem relevant here.

tom-cosgrove-arm
tom-cosgrove-arm approved these changes Aug 3, 2023
View reviewed changes
Copy link
Contributor

@tom-cosgrove-arm tom-cosgrove-arm left a comment •
edited

Choose a reason for hiding this comment

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

Okay with caveat about CIFuzz - and agree urgent

@tom-cosgrove-arm tom-cosgrove-arm removed the approval: review pending This pull request needs review by a committer label Aug 3, 2023
@uudiin uudiin mentioned this pull request Aug 3, 2023
Closed
2 tasks
beldmit
beldmit approved these changes Aug 3, 2023
View reviewed changes
Copy link
Member

@beldmit beldmit left a comment

Choose a reason for hiding this comment

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

Agree urgent, LGTM

@beldmit beldmit added approval: done This pull request has the required number of approvals and removed approval: otc review pending This pull request needs review by an OTC member labels Aug 3, 2023
openssl-machine pushed a commit that referenced this pull request Aug 3, 2023
@hlandau @beldmit
QUIC: Fix incompatible merges causing CI breakage
1e7cc86 
Reviewed-by: Tom Cosgrove <tom.cosgrove@arm.com>
Reviewed-by: Dmitry Belyavskiy <beldmit@gmail.com>
(Merged from #21641)
@beldmit
Copy link
Member

beldmit commented Aug 3, 2023

Merged. Thanks!

@beldmit beldmit closed this Aug 3, 2023
@mattcaswell mattcaswell mentioned this pull request Aug 4, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@beldmit beldmit beldmit approved these changes

@tom-cosgrove-arm tom-cosgrove-arm tom-cosgrove-arm approved these changes

@t8m t8m Awaiting requested review from t8m

@mattcaswell mattcaswell Awaiting requested review from mattcaswell

@paulidale paulidale Awaiting requested review from paulidale

Assignees

@hlandau hlandau

Labels
approval: done This pull request has the required number of approvals branch: master Merge to master branch severity: urgent Fixes an urgent issue (exempt from 24h grace period) tests: exempted The PR is exempt from requirements for testing triaged: bug The issue/pr is/fixes a bug
Projects
Project Board
Status: Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@hlandau @beldmit @tom-cosgrove-arm