Skip to content
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

fix(Dgraph): Replace fmt.Sprintf with strconv.FormatUint int outputrdf.go #6310

Merged
merged 4 commits into from
Sep 1, 2020

Conversation

martinmr
Copy link
Contributor

@martinmr martinmr commented Aug 28, 2020


This change is Reviewable

@martinmr martinmr changed the title Replace fmt.Sprintf with strconv.FormatUint int outputrdf.go fix(Dgraph): Replace fmt.Sprintf with strconv.FormatUint int outputrdf.go Aug 28, 2020
@manishrjain
Copy link
Contributor

This PR would result in more allocations. Have a look at this:

https://github.com/dgraph-io/dgraph/blob/mrjn/fastjson/x/x.go#L1135-L1146

Copy link
Contributor Author

@martinmr martinmr left a comment

Choose a reason for hiding this comment

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

Ok. I am using something similar now.

Reviewable status: 0 of 1 files reviewed, all discussions resolved (waiting on @pawanrawal)

@martinmr martinmr requested a review from a team August 28, 2020 23:09
@parasssh
Copy link
Contributor

CI failed. Please run locally.

@parasssh parasssh merged commit 4b375b3 into master Sep 1, 2020
martinmr added a commit that referenced this pull request Sep 1, 2020
@martinmr martinmr deleted the martinmr/remove-sprintf branch September 1, 2020 17:59
parasssh pushed a commit that referenced this pull request Sep 1, 2020
@martinmr martinmr restored the martinmr/remove-sprintf branch September 1, 2020 18:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants