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

reorganize all tests in a single top folder #3981

Merged
merged 26 commits into from
Sep 25, 2024

Conversation

Lendemor
Copy link
Collaborator

@Lendemor Lendemor commented Sep 24, 2024

No description provided.

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.

this one does kind of scare me a little bit... what if we're not compatible with the latest node version? (idk we don't test it)

@Lendemor
Copy link
Collaborator Author

Understandable, but at the same time, we already have that risk with windows users, so if we ever need to restrain a specific node version, we'll need to hot patch anyway to limit said version.
So making this behaviour the same for all systems does make sense to me ?

Also I think it's probably better to take that risk of a potential risk vs actually preventing some users from using Reflex at all ?

.github/workflows/check_node_latest.yml Outdated Show resolved Hide resolved
.github/workflows/integration_app_harness.yml Outdated Show resolved Hide resolved
.github/workflows/check_node_latest.yml Outdated Show resolved Hide resolved
masenf
masenf previously approved these changes Sep 25, 2024
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.

@picklelo we're eventually moving all tests into subfolders of tests/ if you think that's a good idea

@Lendemor Lendemor changed the title lift node version restraint to allow more recent version if already i… reorganize all tests in a single top folder Sep 25, 2024
@Lendemor Lendemor merged commit 3f53886 into main Sep 25, 2024
39 checks passed
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