-
Notifications
You must be signed in to change notification settings - Fork 7
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
Cat metadata #38
Cat metadata #38
Conversation
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.
removed
47c5638
to
64a3b8f
Compare
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.
Overall this branch is moving in the right direction, but there are a number of improvements I think could be made.
Also FYI, I rebased this branch to incorporate the addition of the build/test pipeline for pull requests in Jenkins. If you log into our Jenkins CI server you should now see pipelines run for this branch/PR. We can cover this more in our next tag-up. Great work so far, keep it up!
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.
The first round of changes look great, but it also looks like a couple things fell through the cracks, and some new issues with the unit tests have been introduced. Getting real close though.
…aned up docstrings.
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.
Branch looks great, thanks again for resolving everything
Basic functionality and unit tests in place.