Skip to content

Conversation

nojaf
Copy link
Contributor

@nojaf nojaf commented Jan 21, 2022

Initial attempt to start with #12418.
I started with SynExprTryWithTrivia, will do more of them once I know I'm on the right track.

Open questions:

  • Do I need a signature file for this?
  • Could potentially always start checking this file with Fantomas (as it is a new file)? Maybe not as part of this PR but to set things up for new files.

Please already review this @dsyme, @auduchinok, @baronfel and others!

Thanks!

@nojaf nojaf force-pushed the trivia branch 2 times, most recently from a89aa35 to 093fe9f Compare January 27, 2022 08:50
@dsyme
Copy link
Contributor

dsyme commented Jan 27, 2022

Do I need a signature file for this?

I guess so, we're using them throughout (just one or two files missing them)

Could potentially always start checking this file with Fantomas (as it is a new file)? Maybe not as part of this PR but to set things up for new files.

We should do that separately

@nojaf nojaf force-pushed the trivia branch 2 times, most recently from 770a724 to 61a2d0c Compare January 27, 2022 16:55
Copy link
Member

@baronfel baronfel left a comment

Choose a reason for hiding this comment

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

minor notes

@nojaf nojaf marked this pull request as ready for review January 28, 2022 10:05
@nojaf
Copy link
Contributor Author

nojaf commented Jan 28, 2022

Hello @dsyme, this is ready for review.
I plan to revisit the other nodes touched by #12311 and #12400 in future PR's, just to keep things manageable.

@nojaf nojaf mentioned this pull request Jan 31, 2022
@dsyme dsyme merged commit a6db54f into dotnet:main Jan 31, 2022
@nojaf nojaf deleted the trivia branch January 31, 2022 17:43
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.

4 participants