Skip to content

Conversation

@dawedawe
Copy link
Contributor

This replaces mkRange for the new range helper functions with the benefit of not going through FileToIndex

@dawedawe dawedawe requested a review from a team as a code owner August 24, 2023 16:37
Copy link
Member

@vzarytovskii vzarytovskii left a comment

Choose a reason for hiding this comment

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

Thanks.
Makes me wonder though, should we, in future, make that module/functions to require RQA, just withStart/withEnd might be a bit confusing for people.

@vzarytovskii vzarytovskii enabled auto-merge (squash) August 25, 2023 10:22
@dawedawe
Copy link
Contributor Author

Thanks. Makes me wonder though, should we, in future, make that module/functions to require RQA, just withStart/withEnd might be a bit confusing for people.

Yes, good call. I can have a go at it sometime next week most likely.

@vzarytovskii vzarytovskii merged commit 443b3ab into dotnet:main Aug 25, 2023
@dawedawe dawedawe deleted the use_range_helpers branch August 28, 2023 16:06
@dawedawe
Copy link
Contributor Author

dawedawe commented Aug 29, 2023

Thanks. Makes me wonder though, should we, in future, make that module/functions to require RQA, just withStart/withEnd might be a bit confusing for people.

@vzarytovskii
For clarification:
As we can't add [<RequireQualifiedAccess>] to single functions, your idea was to add it to the whole Range module, right?
I just want to make sure, because that's going to affect a lot of files and also consumers of FCS.

@vzarytovskii
Copy link
Member

vzarytovskii commented Aug 29, 2023

Thanks. Makes me wonder though, should we, in future, make that module/functions to require RQA, just withStart/withEnd might be a bit confusing for people.

@vzarytovskii
For clarification:
As we can't add [<RequireQualifiedAccess>] to single functions, your idea was to add it to the whole Range module, right?
I just want to make sure, because that's going to affect a lot of files and also consumers of FCS.

Yeah, that's the downside of it. We should probably not change it for now and think more about it.

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

Labels

None yet

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

4 participants