-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
Read schema subfolders #230
Conversation
@lksilva Hey Lucas! Thanks for contributing this! Looks great! The only thing: |
Do you have any suggestion to resolve this issue? That I would be very happy in implement. I apply this suggestion because we need this solution here at work |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@lksilva Yep. Sure! Here's what you'd need to be adjusted.
I made the changes that you requested and I'm thinking about do test to cover FileRepository |
@lksilva Looks great! Could you please rebase your branch, resolve conflicts and fix linter errors? After that we're good to merge. Test would be great! You can add it in this PR or create separate. |
# Conflicts: # packages/cubejs-server-core/core/FileRepository.js
I prefer apply in another PR because I will take a little time for this, can it be like this? |
@lksilva Yep. Sure! Sounds good! |
Check List
Issue Reference this PR resolves
[For example #12]
Description of Changes Made (if issue reference is not provided)
[Description goes here]