Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Add Beintoo adapter #1274
Add Beintoo adapter #1274
Changes from 61 commits
3748413
7f2e8ec
34af94d
2116bc8
5e0994d
f8d9f26
957ee7a
6d2b135
08884dc
19b0e95
dd5190d
eef53fd
d05e82f
2909e88
4f91e27
6cee779
976f223
cbf29d6
d17bcbb
fe92b1b
c5267d7
e486fb9
8274626
b59dca9
91fe738
999db70
85a705e
d77fc77
404e14c
c317042
97b6151
59c663b
b2802bd
61042ff
13569fb
3ffa56d
995759e
1bcc9fc
a34d4a6
124379a
8bf3560
1b31e82
8a8400c
1305fd6
1955e75
d3a089c
b4aa57b
d7cd578
5bbb409
f3c4d23
556d22a
017b5e7
8bd55b2
40ad4dd
bf67959
92b0b7a
006d3d6
9bd5b09
4f20cca
0b04282
0b2befa
fb36e23
80006b2
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
I don't think we need the
bannerCopy
because thebuildImpBanner
already was passed an imp that is a copy, a copy created by thefor
statement in line 181 which is a copy valid in its scope. It doesn't hurt to keep it there, but it'd be a small optimization.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.
I'm sorry @ddantuonobeintoo you were right. This copy is needed. I'm sorry, can we rollback to use it again?
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.
Now that the
banner
copy was removed, the following errors appear becausebanner
should be referenced asimp.Banner
again.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.
Also not that the
Imp
object is local to your adapter, butImp.Banner
is a pointer to the sharedBanner
object. So you will need to make a copy ofBanner
and assign that toImp.Banner
if you want to make any changes toBanner
.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.
Probably we want to call
url.Parse
only once and use thaturl
object for both this and thebuildEndpoint(endpoint string, testing bool, timeout int64)
function.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.
Function
preprocess
callscontinue
everytime it finds an error looking for the good entries in therequest.Imp
array. But theif
statement in line 57 is zero tolerant for any errors making thecontinue
statements unnecessary:Under this logic, we might as well:
Is this what we intended?