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

Adds support for File selector matcher #273

Merged
merged 3 commits into from
Jul 6, 2022

Conversation

drewbanin
Copy link
Contributor

@drewbanin drewbanin commented May 13, 2022

Related to dbt-labs/dbt-core#5241

Description

This PR adds support for the file: selector matcher. The file matcher is implicitly used if the provided selector name ends in a .sql extension (case-insensitive).

Note: One test was failing, though it was unrelated to this change... I need to look into that & update the changelog before merging

Checklist

  • I have signed the CLA
  • I have generated docs locally, and this change appears to resolve the stated issue
  • I have updated the CHANGELOG.md and added information about my change to the "dbt next" section.

@cla-bot cla-bot bot added the cla:yes label May 13, 2022
Copy link

@jwills jwills left a comment

Choose a reason for hiding this comment

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

Thank you Drew! This was neat to learn!

@drewbanin
Copy link
Contributor Author

will merge after #274 is merged & rebased into this branch

@drewbanin drewbanin force-pushed the feature/support-selectors-with-sql-exts branch from afa808f to 9164b1b Compare May 17, 2022 14:23
@drewbanin drewbanin requested review from emmyoop and removed request for jtcohen6 May 17, 2022 14:24
@drewbanin
Copy link
Contributor Author

removed @jtcohen6 and added @emmyoop for review :)

@emmyoop emmyoop requested a review from stu-k May 17, 2022 21:08
Copy link
Contributor

@stu-k stu-k left a comment

Choose a reason for hiding this comment

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

Only some nitpicky, non-blocking comments. I'm not sure of the state of this repo, but I do see some outdated forms of JS, so I'll continue to suggest best practices, if that's okay!

src/app/services/selector_matcher.js Show resolved Hide resolved
src/app/services/selector_matcher.js Show resolved Hide resolved
@emmyoop
Copy link
Member

emmyoop commented Jun 22, 2022

@drewbanin do you think you'll be able to incorporate @stu-k's suggestions so we can include this in the 1.2.0-b1 release?

@jtcohen6
Copy link
Contributor

@emmyoop Given how small the proposed changes are, I think it would make sense for us to take this over the finish line, to ensure it gets in for 1.2.0-rc1 next week.

@jtcohen6 jtcohen6 added the ready_for_review Externally contributed PR has functional approval, ready for code review from Core engineering label Jun 29, 2022
…ure/support-selectors-with-sql-exts

# Conflicts:
#	CHANGELOG.md
@emmyoop emmyoop force-pushed the feature/support-selectors-with-sql-exts branch from 39113c4 to 1f8895e Compare July 6, 2022 15:03
@emmyoop emmyoop merged commit 4d8cc4d into main Jul 6, 2022
@emmyoop emmyoop deleted the feature/support-selectors-with-sql-exts branch September 21, 2022 18:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla:yes ready_for_review Externally contributed PR has functional approval, ready for code review from Core engineering
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants