Skip to content

Added missing fix its #1467

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

Merged
merged 1 commit into from
Apr 15, 2023
Merged

Added missing fix its #1467

merged 1 commit into from
Apr 15, 2023

Conversation

kimdv
Copy link
Contributor

@kimdv kimdv commented Mar 30, 2023

Not sure if this is needed, but I tought it might be a good idea to compare the fixits we expect and what we actually get.

So just got some examples of where it was missing.
There is still a few hundred places missing.

Also tought that I adding fixedSource so we ensure they works in as many scenarios as possible.

@kimdv kimdv force-pushed the kimdv/add-some-fix-its branch from 2032cc1 to 945c466 Compare March 30, 2023 12:38
@kimdv kimdv marked this pull request as ready for review April 2, 2023 09:31
@kimdv kimdv requested a review from ahoppen as a code owner April 2, 2023 09:31
@kimdv kimdv force-pushed the kimdv/add-some-fix-its branch from 945c466 to b95a0b0 Compare April 2, 2023 10:06
@kimdv
Copy link
Contributor Author

kimdv commented Apr 7, 2023

An assertion message could look like testSwitch3(): XCTAssertEqual failed: ("["insert \'{}\'"]") is not equal to ("[]")
Should that be improved ?

@kimdv kimdv force-pushed the kimdv/add-some-fix-its branch from b95a0b0 to 2154f15 Compare April 7, 2023 09:24
@ahoppen
Copy link
Member

ahoppen commented Apr 7, 2023

I think that’s good enough but if you want to improve it, I won’t stop you.

@kimdv kimdv force-pushed the kimdv/add-some-fix-its branch 3 times, most recently from c4b42ba to 146b263 Compare April 9, 2023 16:48
@kimdv kimdv requested a review from ahoppen April 9, 2023 16:48
@kimdv
Copy link
Contributor Author

kimdv commented Apr 9, 2023

@ahoppen we need a bit of planning when merging this.
When it's approved I will rebase on latest main and trigger CI right after.

I think we then should wait a bit on merging other things, to avoid test start failing because of missing fix-its.

@kimdv kimdv force-pushed the kimdv/add-some-fix-its branch from 146b263 to c71230c Compare April 11, 2023 11:11
@kimdv
Copy link
Contributor Author

kimdv commented Apr 11, 2023

@swift-ci please test

@kimdv
Copy link
Contributor Author

kimdv commented Apr 11, 2023

@swift-ci please test windows

@kimdv
Copy link
Contributor Author

kimdv commented Apr 11, 2023

@swift-ci please test Windows

Copy link
Member

@ahoppen ahoppen left a comment

Choose a reason for hiding this comment

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

Thank you. Adding all those expected Fix-Its must have taken you some time 😆 It’s a great change to better testing. I like it.

One minor comment, otherwise you can try landing it over the weekend.

@kimdv kimdv force-pushed the kimdv/add-some-fix-its branch from c61770d to 2358266 Compare April 15, 2023 07:29
@kimdv kimdv force-pushed the kimdv/add-some-fix-its branch from 2358266 to 4357755 Compare April 15, 2023 07:31
@kimdv
Copy link
Contributor Author

kimdv commented Apr 15, 2023

@swift-ci please test

@kimdv
Copy link
Contributor Author

kimdv commented Apr 15, 2023

@swift-ci please test windows

@kimdv kimdv merged commit a0306be into swiftlang:main Apr 15, 2023
@kimdv kimdv deleted the kimdv/add-some-fix-its branch April 15, 2023 15:08
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.

2 participants