-
Notifications
You must be signed in to change notification settings - Fork 842
Cleanup printf #1480
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
Cleanup printf #1480
Conversation
| arr | ||
|
|
||
| member this.GetArgumentAndTypesAsArrays | ||
| member __.GetArgumentAndTypesAsArrays |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@forki @dsyme
I am not a fan of this change, I know you have used it in other cleanup PRs, however, I don't recall a discussion where this was the agreed style.
If we were to agree to a common style for this my preference would be in fact be 'this', since that is what it in fact is. I assume that you want to indicate that the this pointer is unused, but that is not an entirely interesting distinction, given that we are not writing in C# and we are immutable by default. Anyway ... let's have a discussion about a coding standard for this case before we carry on with the clean up.
Kevin
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Obviously I'm pro __ as this is what many projects in the ecosystem use. (also suggested by FSharpLint)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
|
@cloudRoutine For my education, I just checked Visual Studio doesn't do this even at warning level 5 is this a feature of powertools or am I missing something? |
|
@cloudRoutine It's not a feature I would find terribly useful. Although I have to say the whole naming the 'this' seems a bit weird to me as well. I think I would prefer a feature that complained if I named the 'this' differently on any member within the class. I will pull this then since it is a power tools feature, that I suppose many community members have enabled. Despite my personal preferences. Thanks for the clarifications Kevin |
|
@forki Thanks for this contribution Kevin |



No description provided.