Skip to content

Renamed file to fileTree in SourceLocationConverter #1991

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Conversation

natikgadzhi
Copy link
Contributor

@natikgadzhi natikgadzhi commented Aug 3, 2023

Motivation

This pull request closes #1974. My first PR in swift-syntax, just learning my way around things.

Modifications

This pull request renames file to fileName in SourceLocataionConverter so the intent behind it is more readable and obvious. It also provides the new initializer .init(fileName:tree:) and deprecates the existing .init(file:tree:).

TODO

  • Rename file to fileName
  • Make sure tests pass: yep, at least locally.
  • swift-format: is very unhappy, I have a few files diff when I ./format.py.

@natikgadzhi natikgadzhi requested a review from ahoppen as a code owner August 3, 2023 04:33
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.

Thanks @natikgadzhi, these changes look perfect. The formatting is also good, format.py probably through you off because you have swift-format installed on your system. I’m adding a paragraph about it to CONTRIBUTING.md: https://github.com/apple/swift-syntax/pull/1993/files#diff-eca12c0a30e25b4b46522ebf89465a03ba72a03f540796c979137931d8f92055R26

@ahoppen
Copy link
Member

ahoppen commented Aug 3, 2023

I’ll run CI and if it passes, I’ll merge the PR.

@swift-ci Please test

@ahoppen ahoppen enabled auto-merge August 3, 2023 14:58
@natikgadzhi
Copy link
Contributor Author

Yep, I have swift-format from brew — might not be the latest main.

@ahoppen ahoppen merged commit c0c1e5e into swiftlang:main Aug 3, 2023
@natikgadzhi
Copy link
Contributor Author

@ahoppen thank you for such a quick review! 👏🏼

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.

Rename file parameter of SourceLocationConverter initializers to fileName
2 participants