sscheetz
commented
4 years ago Don't worry the PRs I get at work are way bigger than this haha.
Looks good to me. Think the integration test file still has a little more to go before its compatible with the new code, but the unit and keyfile updates are good. Didn't know about that github action thing so thats cool too.
Going to do some very minor follow ups
- you switched assertion error to filenotfound, but I actually shouldn't have commented out the assertion error originally so my fault
- maybe we should assert against the actual count in test_count_saved instead of just asserting thats its truthy
- need to update integration tests rest of the way (and probably commend the create listing one by default because it costs money)
but everything you did is great and I appreciate all the work :) You are obviously more experienced with python than me,
This PR fixes tests, adds GitHub Actions, and fixes a couple of bugs. I know it's a little large, but I tried to keep the commits logical. They mostly don't overlap and can be reviewed individually if that's easier.
I think after this gets merged in, new PRs should automatically show test results. That should look like https://github.com/jtatum/etsy-python2/actions