Skip to content

Expose more services - #8291

Merged
cartermp merged 5 commits into
dotnet:masterfrom
Krzysztof-Cieslak:expose-more-services
Jan 21, 2020
Merged

Expose more services#8291
cartermp merged 5 commits into
dotnet:masterfrom
Krzysztof-Cieslak:expose-more-services

Conversation

@Krzysztof-Cieslak

@Krzysztof-CieslakKrzysztof-Cieslak commented Jan 20, 2020

Copy link
Copy Markdown
Contributor

So this PR does 3 things:

  1. Adds IsPrivateToFile to FSharpSymbolUse - both for VS and FSAC we've been adding it as an extension, but since it's used in both places, I guess we may add it to the type directly
  2. Exposes simplify name analyzer
  3. Exposes unused declarations analyzer

In general, this PR will allow us to remove the whole bunch of code in FSAC that was copy-pasted from here.

cartermp
cartermp suggested changes Jan 20, 2020
View reviewed changes

@cartermpcartermp left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks, this is great! Very happy to have these services/APIs exposed. Some changes are in order here though, since what I think was acceptable as a VFT internal isn't quite good enough for a public API. If you're willing to make those changes/discuss the suggestions then this is definitely mergeable since the actual implementations themselves are fine for exposure.

Comment threadsrc/fsharp/symbols/Symbols.fsi Outdated
Comment threadsrc/fsharp/symbols/Symbols.fs
Comment threadsrc/fsharp/service/ServiceAnalysis.fs Outdated
Comment threadsrc/fsharp/service/ServiceAnalysis.fsi Outdated
Comment threadsrc/fsharp/service/ServiceAnalysis.fs Outdated
Comment threadsrc/fsharp/service/ServiceAnalysis.fs Outdated
Krzysztof-Cieslakand others added 2 commits January 20, 2020 19:13
@Krzysztof-Cieslak@cartermp
Update src/fsharp/symbols/Symbols.fsi
8329021
Co-Authored-By: Phillip Carter <pcarter@fastmail.com>
@Krzysztof-Cieslak
Apply feedback
7813eef
@Krzysztof-Cieslak

Krzysztof-Cieslak commented Jan 20, 2020

Copy link
Copy Markdown
ContributorAuthor

Updated according to the feedback

cartermp
cartermp approved these changes Jan 20, 2020
View reviewed changes

@cartermpcartermp left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks, this is great.

@cartermp
cartermp requested a review from TIHanJanuary 20, 2020 19:44
@cartermp

cartermp commented Jan 20, 2020

Copy link
Copy Markdown
Contributor

@TIHan please review

TIHan
TIHan approved these changes Jan 21, 2020
View reviewed changes

@TIHanTIHan left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good to me.

@cartermp

cartermp commented Jan 21, 2020

Copy link
Copy Markdown
Contributor

let's yeet this puppy into the codebase

@cartermp
cartermp merged commit 1a695c9 into dotnet:masterJan 21, 2020
@Krzysztof-Cieslak
Krzysztof-Cieslak deleted the expose-more-services branch January 22, 2020 00:21
nosami pushed a commit to xamarin/visualfsharp that referenced this pull request Feb 23, 2021
@Krzysztof-Cieslak@cartermp@nosami
Expose more services (dotnet#8291)
a25d03c
* Expose simplify name analyzer
* Move IsPrivateToFile to the FSharpSymbolUse from extensions
* Expose Unused Declarations analyzer
* Update src/fsharp/symbols/Symbols.fsi
Co-Authored-By: Phillip Carter <pcarter@fastmail.com>
* Apply feedback
Co-authored-by: Phillip Carter <pcarter@fastmail.com>
nosami pushed a commit to xamarin/visualfsharp that referenced this pull request Jan 26, 2022
@Krzysztof-Cieslak@cartermp
Expose more services (dotnet#8291)
3166853
* Expose simplify name analyzer
* Move IsPrivateToFile to the FSharpSymbolUse from extensions
* Expose Unused Declarations analyzer
* Update src/fsharp/symbols/Symbols.fsi
Co-Authored-By: Phillip Carter <pcarter@fastmail.com>
* Apply feedback
Co-authored-by: Phillip Carter <pcarter@fastmail.com>
nosami pushed a commit to xamarin/visualfsharp that referenced this pull request Jan 26, 2022
@Krzysztof-Cieslak@cartermp@nosami
Expose more services (dotnet#8291)
08e5c00
* Expose simplify name analyzer
* Move IsPrivateToFile to the FSharpSymbolUse from extensions
* Expose Unused Declarations analyzer
* Update src/fsharp/symbols/Symbols.fsi
Co-Authored-By: Phillip Carter <pcarter@fastmail.com>
* Apply feedback
Co-authored-by: Phillip Carter <pcarter@fastmail.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

2 more reviewers
@TIHanTIHanTIHan approved these changes
@cartermpcartermpcartermp approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@Krzysztof-Cieslak@cartermp@TIHan