Skip to content

[Localization] Make the serialization tool print the removed diagnostics #33337

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

Conversation

HassanElDesouky
Copy link
Contributor

In this PR, I created a vector in swift::diag that will keep track of the diagnostic IDs that got removed from .def and still available in YAML. Then in the swift-serialize-diagnostics I'm printing out all of the removed diagnostic IDs.

cc @xedin

@HassanElDesouky HassanElDesouky requested a review from xedin August 6, 2020 21:18
Copy link
Contributor

@xedin xedin left a comment

Choose a reason for hiding this comment

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

Left a couple of comments inline and this PR needs tests to make sure that swift-serialize-diagnostics behaves as expected.

@HassanElDesouky HassanElDesouky requested a review from xedin August 7, 2020 12:58
@HassanElDesouky HassanElDesouky requested a review from xedin August 7, 2020 22:01
@HassanElDesouky HassanElDesouky requested a review from xedin August 7, 2020 23:34
@HassanElDesouky HassanElDesouky requested a review from xedin August 8, 2020 23:27
@HassanElDesouky HassanElDesouky requested a review from xedin August 9, 2020 19:15
@HassanElDesouky HassanElDesouky requested a review from xedin August 9, 2020 20:26
Copy link
Contributor

@xedin xedin left a comment

Choose a reason for hiding this comment

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

Looks great now! Final set of changes are we are ready to go.

@HassanElDesouky HassanElDesouky force-pushed the localization-report-deleted-IDs branch from 6941ee9 to a3b8014 Compare August 9, 2020 21:19
@HassanElDesouky HassanElDesouky requested a review from xedin August 9, 2020 21:19
@HassanElDesouky
Copy link
Contributor Author

HassanElDesouky commented Aug 9, 2020

I squashed all of the commits, I think it's ready now! :)

Copy link
Contributor

@xedin xedin left a comment

Choose a reason for hiding this comment

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

Looks great, thank you!

@xedin
Copy link
Contributor

xedin commented Aug 9, 2020

@swift-ci please test

@xedin
Copy link
Contributor

xedin commented Aug 9, 2020

@swift-ci please test source compatibility

@HassanElDesouky
Copy link
Contributor Author

Windows failure is related to #33381

@xedin
Copy link
Contributor

xedin commented Aug 10, 2020

@swift-ci please smoke test Windows platform

@xedin xedin merged commit 9642cb6 into swiftlang:master Aug 10, 2020
@HassanElDesouky HassanElDesouky deleted the localization-report-deleted-IDs branch August 14, 2020 01:26
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