Skip to content

Task/andrin/str 154 #14

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

Merged
merged 43 commits into from
Nov 4, 2022
Merged

Task/andrin/str 154 #14

merged 43 commits into from
Nov 4, 2022

Conversation

ocasta181
Copy link
Contributor

@ocasta181 ocasta181 commented Oct 20, 2022

Unit21 Integration layer is now complete.

To test:

Run go test ./pkg/internal/unit21/
Go to https://sandbox2.unit21.com/
Confirm that a new entity, instrument, and transaction were all created at the time you ran the tests. Confirm that each are filled out to a reasonable level of completion. Only missing extraneous details we don't maintain.

@ocasta181 ocasta181 marked this pull request as draft October 20, 2022 22:07
@ocasta181 ocasta181 marked this pull request as ready for review November 1, 2022 21:00
@@ -13,7 +22,7 @@ CREATE TABLE device (
type TEXT DEFAULT '', -- enum: to be defined at struct level in Go
description TEXT DEFAULT '',
fingerprint TEXT DEFAULT '',
ip_addresses JSONB DEFAULT '[]'::JSONB,
ip_addresses TEXT[] DEFAULT NULL,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be JSONB, since we are dealing with it as a string array at the Go level

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the change that @saito-sv and I discussed at some length. It is the 2nd change that will need to be made to all arrays.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@auroter, we can discuss further in office hours.

Copy link

@saito-sv saito-sv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to use the zerolog package for logging but approving here.

@ocasta181 ocasta181 requested a review from alcalawil November 3, 2022 22:15
@ocasta181 ocasta181 merged commit beb2d7c into develop Nov 4, 2022
@ocasta181 ocasta181 deleted the task/andrin/str-154 branch November 4, 2022 00:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants