Skip to content

Conversation

dhruvviee
Copy link
Contributor

Copy link
Member

@ahoppen ahoppen left a comment

Choose a reason for hiding this comment

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

Looks like one case slipped through. Otherwise LGTM. Thanks

Comment on lines 211 to 212
// lookahead + lookaheading on simple if conditions that are obviously
// boolean conditions.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
// lookahead + lookaheading on simple if conditions that are obviously
// boolean conditions.
// lookahead on simple if conditions that are obviously boolean conditions.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you for your suggestion regarding the comments on lines 211 to 212 in Statements.swift. I've made the requested modification as per your recommendation.

@ahoppen
Copy link
Member

ahoppen commented Feb 17, 2024

Oh, one last thing. Could you squash your commits? https://github.com/apple/swift-syntax/blob/main/CONTRIBUTING.md#authoring-commits

Updated comment in Sources/SwiftParser/Statements.swift
@dhruvviee dhruvviee force-pushed the backtrack-lookahead-#2494 branch from 6522f3c to aa0e131 Compare February 17, 2024 04:33
@dhruvviee
Copy link
Contributor Author

Hi, @ahoppen
I've squashed the commits as requested. Thank you for your guidance!

@ahoppen
Copy link
Member

ahoppen commented Feb 19, 2024

@swift-ci Please test

@dhruvviee
Copy link
Contributor Author

@swift-ci @ahoppen
Encountered issue with the test case. I've reviewed the code; can someone please re-run the tests and check the results ?

@ahoppen
Copy link
Member

ahoppen commented Feb 20, 2024

Test failures are unrelated. Let’s try again

@swift-ci Please test

@MaxDesiatov
Copy link
Contributor

@swift-ci test

@ahoppen ahoppen merged commit 57d102f into swiftlang:main Feb 20, 2024
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.

3 participants