Skip to content

Add a new Location.Create overload for external file location with a different mappedLineSpan#64240

Merged
mavasani merged 6 commits intodotnet:mainfrom
mavasani:LocationCreate
Oct 6, 2022
Merged

Add a new Location.Create overload for external file location with a different mappedLineSpan#64240
mavasani merged 6 commits intodotnet:mainfrom
mavasani:LocationCreate

Conversation

@mavasani
Copy link
Contributor

@mavasani mavasani commented Sep 23, 2022

Closes #64236

Proposal details in #64236 (comment)

@ghost ghost added the Area-Analyzers label Sep 23, 2022
@mavasani mavasani mentioned this pull request Sep 23, 2022
Closed
@MariaSolOs MariaSolOs mentioned this pull request Sep 23, 2022
Merged
@mavasani
Merge pull request #2 from MariaSolOs/LocationCreate
114f08f 
Reuse Location constructors and remove FilePath property
@mavasani mavasani marked this pull request as ready for review September 30, 2022 05:21
@mavasani mavasani requested review from a team as code owners September 30, 2022 05:21
@mavasani
Copy link
Contributor Author

mavasani commented Oct 4, 2022

ping @dotnet/roslyn-compiler for reviews.

FYI: This is blocking TypeScript team from moving to a new model of how they report diagnostics with primary/secondary buffer.

@MariaSolOs
Copy link
Contributor

MariaSolOs commented Oct 4, 2022

ping @dotnet/roslyn-compiler for reviews.

FYI: This is blocking TypeScript team from moving to a new model of how they report diagnostics with primary/secondary buffer.

Bumping this. This represents a major change in our script block management and the sooner this change gets merged, the more time we'll have to test this carefully in 17.5.

CyrusNajmabadi
CyrusNajmabadi reviewed Oct 4, 2022
View reviewed changes
@CyrusNajmabadi
Copy link
Contributor

CyrusNajmabadi commented Oct 5, 2022

@dotnet/roslyn-compiler can we get a second pair of eyes on this pr?

AlekseyTs
AlekseyTs reviewed Oct 5, 2022
View reviewed changes
AlekseyTs
AlekseyTs reviewed Oct 5, 2022
View reviewed changes
@AlekseyTs
Copy link
Contributor

AlekseyTs commented Oct 5, 2022

Done with review pass (commit 4)

@MariaSolOs MariaSolOs mentioned this pull request Oct 5, 2022
Merged
@MariaSolOs
Copy link
Contributor

MariaSolOs commented Oct 5, 2022

Thanks @AlekseyTs for the review! I applied your feedback in mavasani#3, (I've been piggy-backing on @mavasani's branch).

@mavasani
Merge pull request #3 from MariaSolOs/LocationCreate
87b918c 
Expose non-mapped location constructor
AlekseyTs
AlekseyTs approved these changes Oct 6, 2022
View reviewed changes
Copy link
Contributor

@AlekseyTs AlekseyTs left a comment

Choose a reason for hiding this comment

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

LGTM (commit 6). Please squash commit in the process of merging.

@mavasani mavasani enabled auto-merge (squash) October 6, 2022 01:07
@mavasani mavasani merged commit 80277a5 into dotnet:main Oct 6, 2022
@ghost ghost added this to the Next milestone Oct 6, 2022
@mavasani mavasani deleted the LocationCreate branch October 6, 2022 02:19
@allisonchou allisonchou mentioned this pull request Oct 11, 2022
Closed
@ryzngard ryzngard mentioned this pull request Oct 19, 2022
Merged
@RikkiGibson RikkiGibson removed this from the Next milestone Oct 24, 2022
@RikkiGibson RikkiGibson added this to the 17.5 P1 milestone Oct 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@333fred 333fred 333fred approved these changes

@AlekseyTs AlekseyTs AlekseyTs approved these changes

+2 more reviewers

@CyrusNajmabadi CyrusNajmabadi CyrusNajmabadi left review comments

@MariaSolOs MariaSolOs MariaSolOs left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

Area-Analyzers

Projects

None yet

Milestone

17.5 P1

Development

Successfully merging this pull request may close these issues.

Add a new Location.Create overload for external file location with a different mappedLineSpan

6 participants

@mavasani @MariaSolOs @CyrusNajmabadi @AlekseyTs @333fred @RikkiGibson