k-capehart / go-salesforce

Salesforce REST API client written in Go
https://pkg.go.dev/github.com/k-capehart/go-salesforce/v2
MIT License
24 stars 4 forks source link

feature/return sf result #53

Closed k-capehart closed 3 weeks ago

k-capehart commented 1 month ago

addresses: #42 continues work from: https://github.com/k-capehart/go-salesforce/pull/45

TODO:

k-capehart commented 1 month ago

@iamber12 @0cv I'd appreciate both your eyes on this if you have time over the next week or two. It continues the work from #45 and would be part of a v2.0 release.

I'll be out for a couple weeks so no rush on it. I'll be able to return to it after but let me know if anything jumps out to you as a bad idea.

0cv commented 1 month ago

@k-capehart 2 days ago, I've swapped in my app my own PR by the latest commit of this PR and this seems to be working as before 👍

codecov-commenter commented 1 month ago

:warning: Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

Attention: Patch coverage is 87.25490% with 26 lines in your changes missing coverage. Please review.

Project coverage is 86.45%. Comparing base (32f00cf) to head (92f2e72).

Files Patch % Lines
dml.go 81.81% 10 Missing and 4 partials :warning:
composite.go 83.33% 7 Missing :warning:
salesforce.go 89.74% 3 Missing and 1 partial :warning:
bulk.go 97.72% 1 Missing :warning:

:exclamation: Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #53 +/- ## ========================================== - Coverage 87.13% 86.45% -0.68% ========================================== Files 6 6 Lines 1003 997 -6 ========================================== - Hits 874 862 -12 - Misses 65 69 +4 - Partials 64 66 +2 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

k-capehart commented 3 weeks ago

@ccoVeille Since you're reviewing, how do you feel about returning the SalesforceResult and SalesforceResults as pointers? That's how I wrote it in this PR initially but now I can't think of any good reason why that's better than just returning the value. Any opinions?

ccoVeille commented 3 weeks ago

I'm fine with pointer or not.

The first one is supposed to avoid using more memory.

But I'm unsure when the result is a map or a slice.

A benchmark would help here maybe