-
Notifications
You must be signed in to change notification settings - Fork 207
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
Amend list initialization sequence #581
Amend list initialization sequence #581
Conversation
i-garrison
commented
Oct 7, 2023
- add more tests for list-initialization of class or aggregate
- amend list-initialization handling of class or aggregate
Hi @jonahgraham - hope this CI build issue is transient?
|
37d1c8b
to
7a77945
Compare
There was a missing apt update in our script - fixed in #586 - please rebase changes once I merge #586 to pick up the fix. |
(My commit message in #586 referenced this PR in a way that caused it to be auto-closed. Re-opening) |
90c076b
to
21bdbb1
Compare
Hi @jonahgraham would you believe we can get this merged before 11.4.0 cutoff date? Mind that #448 is conflicting with his one but we have an agreement to prefer this (larger) change #448 (comment) |
21bdbb1
to
45c3ea0
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At recent CDT calls we discussed how to handle reviews where there is insufficient expertise to review amongst existing committers and decided to accept such changes that look reasonable and appear sufficiently tested.
I have pushed a rebased version of this change.
45c3ea0
to
5676220
Compare
I bumped the cdt.ui and updated the dep on cdt core to make sure the method with the new parameter is properly picked up. |