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

Remove dead resolve code #1652

Merged
merged 4 commits into from
Jan 18, 2024
Merged

Remove dead resolve code #1652

merged 4 commits into from
Jan 18, 2024

Conversation

Ekrekr
Copy link
Contributor

@Ekrekr Ekrekr commented Jan 17, 2024

No description provided.

@Ekrekr Ekrekr requested a review from BenBirt January 17, 2024 18:36
@Ekrekr
Copy link
Contributor Author

Ekrekr commented Jan 18, 2024

I don't think it makes sense to have the supportSqlFileCompilation in v3, it overloads the loading of SQL files from with actionConfigs

@BenBirt
Copy link
Collaborator

BenBirt commented Jan 18, 2024

I don't think it makes sense to have the supportSqlFileCompilation in v3, it overloads the loading of SQL files from with actionConfigs

I agree - it is part of the reason I wanted to the merge to happen

core/session.ts Outdated
)
);
this.compileError(new Error(`Could not resolve ${JSON.stringify(ref)}`));
return "unresolved";
Copy link
Collaborator

Choose a reason for hiding this comment

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

please continue to return an empty string, as we do above for similar cases.

@Ekrekr Ekrekr merged commit 4987628 into main_v3 Jan 18, 2024
4 checks passed
@Ekrekr Ekrekr deleted the remove-dead-resolve-code branch January 18, 2024 14:27
moker-spaghetti pushed a commit to moker-spaghetti/dataform that referenced this pull request May 26, 2024
* Remove dead resolve code

* Tidy

* Make returned string empty rather than unresolved
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.

2 participants