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: Adding var template support #3053

Merged
merged 5 commits into from Aug 15, 2023
Merged

feat: Adding var template support #3053

merged 5 commits into from Aug 15, 2023

Conversation

xoscar
Copy link
Collaborator

@xoscar xoscar commented Aug 9, 2023

This PR updates the app to support the var keyword as part of triggers and test specs

Changes

  • Updates lang parsers to support var
  • Changes backend to use the same data store and behavior as for env
  • Updates auto-complete options to show variable set values

Fixes

Checklist

  • tested locally
  • added new dependencies
  • updated the docs
  • added a test

https://www.loom.com/share/a9d1958fe876417a9d38e42c4788878a

@xoscar xoscar changed the title feat: Adding template support feat: Adding var template support Aug 10, 2023
@xoscar xoscar marked this pull request as ready for review August 10, 2023 15:56
ExpectedErrorMessage: `resolution error: environment variable "test" not found`,
ExpectedErrorMessage: `resolution error: variable "test" not found`,

EnvironmentDataStore: expression.EnvironmentDataStore{
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this attribute be VariableSetDataStore?

Copy link
Contributor

@jorgeepc jorgeepc left a comment

Choose a reason for hiding this comment

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

💥

@xoscar xoscar self-assigned this Aug 10, 2023
@@ -99,5 +99,5 @@ func (ds EnvironmentDataStore) Get(name string) (string, error) {
}
}

return "", fmt.Errorf(`environment variable "%s" not found`, name)
return "", fmt.Errorf(`variable "%s" not found`, name)
Copy link
Member

Choose a reason for hiding this comment

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

We should rename the EnvironmentDataStore to VariableSetDataStore or something similar

Copy link
Member

Choose a reason for hiding this comment

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

Rest looks good

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done! Do you mind retaking a look?

@xoscar xoscar merged commit abd131a into main Aug 15, 2023
30 checks passed
@xoscar xoscar deleted the feat/var-template-support branch August 15, 2023 18:00
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