Skip to content

Handle all FileTypes in OutputFileMap #35

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

sjavora
Copy link
Contributor

@sjavora sjavora commented Nov 15, 2019

This replaces the specific properties in Entry with a dictionary keyed by FileType and adds a custom implementation of Codable.

Originally, I wanted to just use FileType's description property, but some of the cases just return rawValue, which is not compatible in all cases ("o" vs "object", in particular). Instead I added the name property with values from FileTypes.def. While there, I noticed that FileType doesn't currently have a case for swiftsourceinfo, so I added that as well.

@sjavora sjavora changed the title Handle all FileTypes in OutputFileMap Handle all FileTypes in OutputFileMap Nov 15, 2019
@sjavora
Copy link
Contributor Author

sjavora commented Nov 19, 2019

Hey @DougGregor - could someone please take a look at this? I'd request a review, but the button doesn't seem to be there...

@harlanhaskins
Copy link
Contributor

The approach looks good here, I've got one small nitpick though.

@sjavora sjavora force-pushed the there-can-be-only-one-filetype branch from dcf1bc8 to 4ceee16 Compare November 20, 2019 06:31
Copy link
Contributor

@harlanhaskins harlanhaskins left a comment

Choose a reason for hiding this comment

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

LGTM!

@harlanhaskins
Copy link
Contributor

@swift-ci please test

@harlanhaskins
Copy link
Contributor

@swift-ci please test

@harlanhaskins harlanhaskins merged commit fda643c into swiftlang:master Nov 22, 2019
@harlanhaskins
Copy link
Contributor

Thanks, @sjavora!

@sjavora sjavora deleted the there-can-be-only-one-filetype branch February 6, 2021 12:48
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