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

Remove unncessary conversions #315

Merged
merged 1 commit into from
Jul 12, 2017
Merged

Remove unncessary conversions #315

merged 1 commit into from
Jul 12, 2017

Conversation

tamird
Copy link
Contributor

@tamird tamird commented Mar 17, 2017

Found with github.com/mdempsky/unconvert.

/Users/tamird/src/go/src/github.com/golang/protobuf/jsonpb/jsonpb.go:625:32: unnecessary conversion
/Users/tamird/src/go/src/github.com/golang/protobuf/proto/encode.go:177:30: unnecessary conversion
/Users/tamird/src/go/src/github.com/golang/protobuf/proto/encode.go:181:26: unnecessary conversion
/Users/tamird/src/go/src/github.com/golang/protobuf/proto/text_parser.go:868:21: unnecessary conversion

@LMMilewski
Copy link
Member

LMMilewski commented Mar 21, 2017

Hi Tamir. Thank you for this and other PRs. I think that those are good changes and I'd like to accept them. That said, we are currently working on improving performance of the proto package. Those changes are extensive and I'd like to push them before accepting other PRs to minimize the amount of work that needs to be done. Is waiting a few weeks ok with you?

@tamird
Copy link
Contributor Author

tamird commented Mar 28, 2017

Hi @LMMilewski. Sounds like a classic response on a Google OSS project - please wait an indeterminate period of time on this 4-line change so that our developers can avoid learning how to use Git.

See also:
#37
#39
#40
#78

@tamird
Copy link
Contributor Author

tamird commented Jul 11, 2017

@LMMilewski it's been 3 months. Consider merging this, please?

@cybrcodr
Copy link
Contributor

Hi @tamird. Can you sync the PR to head? And I can merge it in. I may not be able to quickly respond but will try to get this in sometime this week.

I've not looked at your other PRs, but do sync them to head as well and I can take a look. We've been holding off some of these PRs as we do need to sync them with another repo and it's a bit challenging at times to sync both ways.

Thanks.

Found with github.com/mdempsky/unconvert.

	/Users/tamird/src/go/src/github.com/golang/protobuf/jsonpb/jsonpb.go:625:32: unnecessary conversion
	/Users/tamird/src/go/src/github.com/golang/protobuf/proto/encode.go:177:30: unnecessary conversion
	/Users/tamird/src/go/src/github.com/golang/protobuf/proto/encode.go:181:26: unnecessary conversion
	/Users/tamird/src/go/src/github.com/golang/protobuf/proto/text_parser.go:868:21: unnecessary conversion
@tamird
Copy link
Contributor Author

tamird commented Jul 11, 2017 via email

@cybrcodr cybrcodr merged commit 0a4f71a into golang:master Jul 12, 2017
@tamird tamird deleted the unconvert branch July 12, 2017 04:23
@golang golang locked and limited conversation to collaborators Jun 26, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants