Skip to content

Conversation

@VitaliyKurokhtin
Copy link
Contributor

Is there a reason to have Diagnostic on ParseNode rather than in context as in this PR? Judging from code they both go hand in hand always and I can't really see any valuable scenario when one would need to change diagnostic between nodes.

Copy link
Member

@darrelmiller darrelmiller left a comment

Choose a reason for hiding this comment

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

This looks like a great change to me. Thanks! I have a feeling that context got introduced after diagnostic and we just never noticed that context could carry it.

@darrelmiller darrelmiller added this to the 1.1.5 milestone Dec 7, 2019
@darrelmiller darrelmiller changed the base branch from master to vnext December 16, 2019 00:59
@darrelmiller darrelmiller merged commit abfebb1 into microsoft:vnext Dec 16, 2019
@VitaliyKurokhtin VitaliyKurokhtin deleted the vvk/diagnostic-in-context branch February 19, 2020 01:46
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.

2 participants