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

[TypeChecker] <unknown> diagnostic location regarding Codable derived conformances #31037

Merged
merged 6 commits into from Apr 15, 2020

Conversation

HassanElDesouky
Copy link
Contributor

Fixing diagnostic location regarding Codable derived conformances.

rdar://problem/59655704
Resolves SR-12248.

@HassanElDesouky
Copy link
Contributor Author

@CodaFi Please review. And sorry for the inconvenience :)

@HassanElDesouky
Copy link
Contributor Author

@CodaFi when this gets merged I'll open a PR for removing the setInvalids()s

@CodaFi
Copy link
Member

CodaFi commented Apr 15, 2020

Don't worry about it. We all miss tests all the time

@swift-ci smoke test

@CodaFi
Copy link
Member

CodaFi commented Apr 15, 2020

For reference, you can always rebase and force-push at your original PR instead of opening a new one if stuff like this arises.

Also making a note here that I'm going to squash this patch down manually once it passes. In the future, could you make sure to use

git pull --rebase <remote> <branch>

Instead of a normal git pull? That'll ensure there aren't any intervening merge commits which keeps the history somewhat more tidy.

@HassanElDesouky
Copy link
Contributor Author

@CodaFi Thank you for the heads up! I really appreciate it.

It passed the tests now :)

@CodaFi
Copy link
Member

CodaFi commented Apr 15, 2020

@CodaFi CodaFi merged commit f4006c3 into apple:master Apr 15, 2020
@CodaFi
Copy link
Member

CodaFi commented Apr 15, 2020

Thanks. Don't forget to resolve the SR!

@HassanElDesouky
Copy link
Contributor Author

Thanks. Don't forget to resolve the SR!

Of course! Thank you again 🙏

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.

None yet

2 participants