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

feat: dynamic case label #236

Merged
merged 6 commits into from Oct 22, 2019
Merged

feat: dynamic case label #236

merged 6 commits into from Oct 22, 2019

Conversation

ferrants
Copy link
Contributor

@ferrants ferrants commented Oct 22, 2019

Check List

  • Tests has been run in packages where changes made if available
  • Linter has been run for changed code
  • Tests for the changes have been added if not covered yet
  • Docs have been added / updated if required

Issue Reference this PR resolves

#235

Description of Changes Made (if issue reference is not provided)

label is run through the execute sql function

@paveltiunov
Copy link
Member

@ferrants Thanks for contributing this! So the implementation can be just to check if label is an object with sql property then we can evaluate it otherwise save old behavior. Need to add appropriate checks in CubeValidator as well. Test for this use case can be very helpful here.

@ferrants ferrants changed the title Make label dynamic feat: dynamic case label Oct 22, 2019
@ferrants
Copy link
Contributor Author

ferrants commented Oct 22, 2019

Linter has been run for changed code

@paveltiunov I could add linting to this package, but it'll make the PR much bigger.

@paveltiunov
Copy link
Member

@ferrants schema compiler isn't ready for whole package linting so let's keep it as is.

Overall PR looks great! Just remove this orphaned res = and we're good to merge! Thanks again!

@paveltiunov paveltiunov merged commit 1a82605 into cube-js:master Oct 22, 2019
@ferrants ferrants deleted the patch-1 branch October 22, 2019 22:17
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

2 participants