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.

Sign up for GitHub

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

Jump to bottom

use add_imports everywhere #3448

Merged
merged 10 commits into from
Jun 12, 2024
Merged

use add_imports everywhere #3448

merged 10 commits into from
Jun 12, 2024

Conversation

Lendemor
Copy link
Collaborator

@Lendemor Lendemor commented Jun 5, 2024 •
edited
Loading

Replace use of _get_imports with the new API add_imports.

Also tried to unify the typing for the message by using ImportDict and ParsedImportDict types.

@Lendemor Lendemor marked this pull request as ready for review June 6, 2024 19:22
@Lendemor Lendemor requested a review from masenf June 6, 2024 19:45
masenf
masenf reviewed Jun 7, 2024
View reviewed changes
Copy link
Collaborator

@masenf masenf left a comment

Choose a reason for hiding this comment

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

Mostly looks right, i have yet to check it out and test it out locally though

reflex/compiler/utils.py Outdated Show resolved Hide resolved
reflex/vars.py Outdated Show resolved Hide resolved
@Lendemor
Copy link
Collaborator Author

Lendemor commented Jun 7, 2024

Mostly looks right, i have yet to check it out and test it out locally though

Addressed your two comment, I also added unit tests for the new parse_imports function

masenf
masenf reviewed Jun 10, 2024
View reviewed changes
reflex/components/component.py Outdated Show resolved Hide resolved
reflex/vars.py Outdated Show resolved Hide resolved
@Lendemor Lendemor requested a review from masenf June 11, 2024 15:00
@picklelo picklelo merged commit 462b023 into main Jun 12, 2024
47 checks passed
@masenf masenf deleted the lendemor/use_add_imports branch December 12, 2024 07:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@masenf masenf masenf approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Lendemor @masenf @picklelo