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

Update _gLobal_config_checker.py #65

Merged
merged 5 commits into from
May 29, 2023
Merged

Conversation

bobbyshermi
Copy link
Contributor

@bobbyshermi bobbyshermi commented May 17, 2023

Issue 61 on taipy-config

Add a warning checker to verify the value of the repository_type is either sql or filesystem.

Add a warning checker to verify the value of the repository_type
Copy link
Member

@trgiangdo trgiangdo left a comment

Choose a reason for hiding this comment

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

Hello @bobbyshermi, thank you for your contribution to Taipy. We are really appreciate your help.

Could you please add a test case to the test_gLobal_config_checker.py so we can run the test and make sure everything is correct on all environments?

Otherwise, I only have some small comments on the code.

bobbyshermi and others added 4 commits May 18, 2023 10:24
Add tests on global config checker
Update test_gLobal_config_checker.py
Co-authored-by: Đỗ Trường Giang <dtr.giang.1299@gmail.com>
@bobbyshermi
Copy link
Contributor Author

Thank you for the review. I updated the checker to match your remarks and I created some unit tests.

Copy link
Member

@jrobinAV jrobinAV left a comment

Choose a reason for hiding this comment

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

It looks good to me. Thank you very much.

FYI, we target to add it in the release 2.3 planned for June.
But merging it will require some work on the enterprise version on our side. I hope we will have time to make it before the release.
We let you know anyway.

Thank you again.

Copy link
Member

@toan-quach toan-quach left a comment

Choose a reason for hiding this comment

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

@bobbyshermi
Love what you did! Thank you for contributing to Taipy!!
I think we're ready to merge this ticket!! Though there's some minor proposal from me (inspired by @trgiangdo) to make it easier for expansion and maintenance. Let me know what you think!!!

Copy link
Member

@toan-quach toan-quach left a comment

Choose a reason for hiding this comment

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

@bobbyshermi
I think we're ready to merge this ticket!! Though there's some minor proposal from me (inspired by @trgiangdo) to make it easier for expansion and maintenance. Let me know what you think!!!

@toan-quach toan-quach merged commit 4af3895 into Avaiga:develop May 29, 2023
27 of 28 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.

None yet

4 participants