Skip to content

Conversation

@edgarfgp
Copy link
Contributor

@edgarfgp edgarfgp commented Sep 22, 2022

Fixes #13148

Update: I have decided to not fix #13981 as per of this PR as this issue seems t be more complex than what I thought at the beginning

@edgarfgp edgarfgp marked this pull request as ready for review September 23, 2022 04:05
@edgarfgp
Copy link
Contributor Author

@vzarytovskii I think this is ready :) . Let me know if you are happy with the error message .

…eters-with-the-same-name-in-abstract-methods
Copy link
Contributor

@dsyme dsyme left a comment

Choose a reason for hiding this comment

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

Just some minor de-functionalization requested please

Copy link
Contributor

@dsyme dsyme left a comment

Choose a reason for hiding this comment

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

Signature files noted

dsyme and others added 3 commits September 24, 2022 12:46
…eters-with-the-same-name-in-abstract-methods
…the-same-name-in-abstract-methods' of https://github.com/edgarfgp/fsharp into compiler-error-when-declaring-multiple-parameters-with-the-same-name-in-abstract-methods
Copy link
Contributor

@dsyme dsyme left a comment

Choose a reason for hiding this comment

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

This is great but please factor out the common code. I'd like us to start being more rigorous about avoiding code duplication like this in the compiler, when it's easy to do so.

@edgarfgp
Copy link
Contributor Author

This is great but please factor out the common code. I'd like us to start being more rigorous about avoiding code duplication like this in the compiler, when it's easy to do so.

Thanks . I will be more rigorous in my next contributions . I appreciate the patience you have :)

@edgarfgp edgarfgp requested a review from dsyme September 30, 2022 08:49
…eters-with-the-same-name-in-abstract-methods

# Conflicts:
#	src/Compiler/FSComp.txt
…eters-with-the-same-name-in-abstract-methods
@edgarfgp edgarfgp requested review from vzarytovskii and removed request for dsyme October 3, 2022 17:24
@edgarfgp
Copy link
Contributor Author

edgarfgp commented Oct 5, 2022

@dsyme In my last commit I have added also a fix for #13981 as this PR is fixing a similar issue, But I'm not sure about the error message that I'm reusing. Let me know if you have a better message that we use :)

@edgarfgp edgarfgp closed this Oct 11, 2022
@edgarfgp edgarfgp deleted the compiler-error-when-declaring-multiple-parameters-with-the-same-name-in-abstract-methods branch October 11, 2022 17:43
@edgarfgp
Copy link
Contributor Author

Closing this PR :). As I had a CI error with the generated FSComp.txt strings :( . I will create a new PR in a bit

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

5 participants