-
Notifications
You must be signed in to change notification settings - Fork 9
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: Add override for test groups #2335
Conversation
joshlarson
commented
Dec 21, 2023
•
edited
Loading
edited
- To see the specific tasks where the Asana app for GitHub is being used, see below:
- https://app.asana.com/0/0/1206195348204443
Coverage of commit
|
end | ||
|
||
def all_test_group_names(user) do | ||
user = Skate.Repo.preload(user, :test_groups) |
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.
I would eventually like for this to be just one query, but I wasn't convinced that that was necessary right now. Unless there's something about Ecto that I'm missing, this will run 2 queries, not like.. 20.
Coverage of commit
|
9a58295
to
769e464
Compare
use Ecto.Migration | ||
|
||
def change do | ||
# excellent_migrations:safety-assured-for-this-file raw_sql_executed |
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.
Just our luck - the first migration we write with excellent_migrations
in place, and we basically have to ignore it 🤦
Coverage of commit
|
Coverage of commit
|
Coverage of commit
|
priv/repo/migrations/20231220204054_add_override_for_test_groups.exs
Outdated
Show resolved
Hide resolved
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.
Okay done with a first pass review. I think the three things that need addressing are the default value of the column, the name of get_enabled
, and the skipped test.
Coverage of commit
|
Coverage of commit
|
Coverage of commit
|
Coverage of commit
|
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.
Looks good to me and I was playing around with it locally and everything seems to work as expected. 👍
b11892b
to
e11f227
Compare
Coverage of commit
|
Coverage of commit
|
e11f227
to
f232a45
Compare
Coverage of commit
|
f232a45
to
7f73299
Compare
Coverage of commit
|
Coverage of commit
|