- User Since
- Jul 8 2019, 5:19 PM (51 w, 3 d)
Woah this cleaned up nicely, great job! Glad to see the extra boolean flag and conditional mark go away.
Nice nice 👍
hmm, can you just move all of the other dagster tests under dagster_tests/general_tests? then you shouldn't need the ignores, right?
Nice work! It’s great to see the graphql tests finally run against Buildkite.
Wed, Jul 1
Tue, Jun 30
Mon, Jun 29
As discussed on Slack, might be worth to get rid of the scrub_key_from_dict helper and just do the dummy value replacement inline. Otherwise, looks great!
Sun, Jun 28
Fri, Jun 26
Awesome job consolidating these – I can tell it wasn't easy. Added an inline question about the use of the snapshot_friendly flag.
Thu, Jun 25
Yeah good point. In reality they are pretty connected though, so I'll form the connection in the next rev
fix lazy loading
I think this is a more than solid first pass, and can address schrock's comments in the next one. Would be great to get this landed for today's release.
Looks great! Solid, self contained example with the DBs 👍
Address feedback, fix lint errors, add tests
You may want to add a simple # TODO: remove dependency on legacy_examples inline right before the issue message.
Looks great. These tests are supposed to show users how to test their code, so it's worth cleaning them up. What do you think about just checking pipeline success status, rather than checking that each solid was successful?
Wed, Jun 24
Tue, Jun 23
My bad - must have been a bad amend
Looks good. Did you see any staleness issues?
This is great! Nice to see all those protected accesses go away.