Closed pnadolny13 closed 2 years ago
In GitLab by @hsyyid on Jun 17, 2021, 10:42
added 1 commit
In GitLab by @hsyyid on Jun 17, 2021, 10:47
added 3 commits
In GitLab by @hsyyid on Jun 17, 2021, 10:48
added 1 commit
In GitLab by @DouweM on Jun 17, 2021, 11:01
@aaronsteers Please review! Once merged, don't forget to bump the version, update the changelog, and create a tag.
In GitLab by @DouweM on Jun 17, 2021, 11:01
assigned to @aaronsteers
In GitLab by @aaronsteers on Jun 23, 2021, 16:00
@hsyyid - I've completed a review of the source code changes and they look good. Since we don't yet have automated tests and/or a CI pipeline for this repo, do you mind letting me know if you have had successful test results already on your side, and if so, which tests you were able to run successfully?
Thanks very much!
In GitLab by @aaronsteers on Jun 23, 2021, 16:01
requested review from @aaronsteers
In GitLab by @hsyyid on Jun 23, 2021, 16:18
@aaronsteers I didn't test with the test cases in there, but I did run the tap locally with my use case again and it's working without issues. I can try running the test cases if you'd like
In GitLab by @aaronsteers on Jun 25, 2021, 13:32
@hsyyid Do we even have test cases yet? I didn't see any so I assume we probably have to work with manual test runs for the time being.
Just knowing you were able to run locally is a big help. I'll take a final pass today and Monday and will post any other feedback/requests.
Thanks!
In GitLab by @hsyyid on Jul 19, 2021, 12:42
@aaronsteers Just checking up here – to confirm, this runs locally without issue.
Thanks!
In GitLab by @aaronsteers on Jul 19, 2021, 13:29
@hsyyid - Thanks for confirming. I spent a little more time reviewing and it looks good. Thanks very much for this contribution!
In GitLab by @aaronsteers on Jul 19, 2021, 13:29
approved this merge request
In GitLab by @aaronsteers on Jul 19, 2021, 13:29
mentioned in commit b8d45d1c3068579799c35a01dd2717d2b9393de0
In GitLab by @hsyyid on Jun 17, 2021, 10:25
Merges fix/missing-schema-entries -> master