Skip to content

Conversation

@edgarfgp
Copy link
Contributor

Now that #13700 merged. As part of this PR we will update FSharp.Core

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

@vzarytovskii I have updated for now only array.fs . Not sure if we want to update all of the instances where we use [] to use array ? . Happy to do it as part of another PR :)

@KevinRansom
Copy link
Contributor

@edgarfgp -
If we think this is the correct way to format array type definitions, wouldn't it be preferable to implement them using fantomas so that we can police it, rather than providing a thousand manually edited PR's?

@vzarytovskii
Copy link
Member

Hm, to be completely honest, I'm not sure we should update everything now.

As per fsharp/fslang-suggestions#635 (comment) - the most important places are tooling presentation (addressed in the original PR), API reference and docs.

@vzarytovskii
Copy link
Member

@edgarfgp -
If we think this is the correct way to format array type definitions, wouldn't it be preferable to implement them using fantomas so that we can police it, rather than providing a thousand manually edited PR's?

Yep, agree, I think if we want to unify it, we should enforce it.

@edgarfgp
Copy link
Contributor Author

@vzarytovskii @KevinRansom Thanks for the feedback. Agreed Fantomas is a better approach to do this :) . happy to help either way . Closing this PR

@edgarfgp edgarfgp closed this Sep 17, 2022
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.

3 participants