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

Add EnumParam preserving type information #1266

Merged
merged 2 commits into from Dec 12, 2019

Conversation

gsomix
Copy link
Contributor

@gsomix gsomix commented Oct 2, 2019

Closes #1262

I've added new type for enum params, that preserves type information for erasured F# enums in attributes. There is obvious code duplication, but that's for reason to keep code changes small and local. We can remove it after F# compiler is fixed. ;)

  • Fix F# benchmarks compilation with enum params
  • Fix enum values names in Summary
  • Write tests for this case

See also:
dotnet/fsharp#995

Happy #hacktoberfest!

@gsomix
Copy link
Contributor Author

gsomix commented Oct 2, 2019

I'd like to write proper tests for this case, but it doesn't clear how to run F# integration "test suite". Could you help me please? @adamsitnik @AndreyAkinshin Thanks!

@gsomix
Copy link
Contributor Author

gsomix commented Oct 2, 2019

@isaevdan Review please!

Copy link

@isaevdan isaevdan left a comment

Choose a reason for hiding this comment

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

Seems fine by me

@gsomix gsomix changed the title [WIP] Add EnumParam preserving type information Add EnumParam preserving type information Oct 3, 2019
@gsomix
Copy link
Contributor Author

gsomix commented Oct 7, 2019

Ready to review. :)

@isaevdan
Copy link

I see this is still open. In case if it is waiting for me, I`m still fine with implementation :)

@gsomix
Copy link
Contributor Author

gsomix commented Dec 12, 2019

@adamsitnik @AndreyAkinshin Hey, guys, any chance to get it merged? Thanks!

@AndreyAkinshin AndreyAkinshin merged commit 55842bf into dotnet:master Dec 12, 2019
@AndreyAkinshin
Copy link
Member

@gsomix thanks for the PR and sorry for the delay.

@AndreyAkinshin AndreyAkinshin added this to the v0.12.1 milestone Dec 12, 2019
@isaevdan
Copy link

isaevdan commented Mar 30, 2020

@AndreyAkinshin @gsomix
Am I correct that this is no available yet in 0.12, and will be in 0.12.1?
Is there any plans when 0.12.1 will be released?

@AndreyAkinshin
Copy link
Member

@isaevdan yes, the fix will be available in 0.12.1. The new version should be released this week.

@isaevdan
Copy link

@AndreyAkinshin Great, thanks for update

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.

Params attribute doesn`t work in F# if you specify more than one enum value in constructor
3 participants