Skip to content
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

Rename PreorderVisitor to SourceOrderVisitor #11798

Merged
merged 2 commits into from
Jun 7, 2024
Merged

Conversation

MichaReiser
Copy link
Member

Summary

There has been some confusion about the order in which PreoderVisitor visits the AST nodes because it is poorly named (I'm allowed to say this, I wrote it ;))

This PR renames the PreorderVisitor to SourceOrderVisitor and hints users in the documentation to to Visitor that does evaluation order visiting.

Test Plan

cargo test

@MichaReiser MichaReiser added the internal An internal refactor or improvement label Jun 7, 2024
Copy link
Contributor

github-actions bot commented Jun 7, 2024

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

Formatter (stable)

✅ ecosystem check detected no format changes.

Formatter (preview)

✅ ecosystem check detected no format changes.

Copy link
Member

@AlexWaygood AlexWaygood left a comment

Choose a reason for hiding this comment

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

Nice, this is much clearer IMO

crates/ruff_python_ast/src/visitor/source_order.rs Outdated Show resolved Hide resolved
Copy link
Contributor

@carljm carljm left a comment

Choose a reason for hiding this comment

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

Looks great!

Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
@MichaReiser MichaReiser enabled auto-merge (squash) June 7, 2024 16:58
@MichaReiser MichaReiser merged commit 32ca704 into main Jun 7, 2024
16 checks passed
@MichaReiser MichaReiser deleted the rename-preorder-visitor branch June 7, 2024 17:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
internal An internal refactor or improvement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants