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

Update opentelemetry-proto submodule to version from May 18, 2021 #825

Merged
merged 6 commits into from
Jun 7, 2021

Conversation

maxgolov
Copy link
Contributor

@maxgolov maxgolov commented Jun 4, 2021

Update opentelemetry-proto submodule from 5 months old to latest version:

commit 795cc815ce9bb438b46a4a7f6ecb465b52d50935 (HEAD -> main, origin/main, origin/HEAD)
Author: Bogdan Drutu <bogdandrutu@gmail.com>
Date:   Tue May 18 14:42:43 2021 -0700

No other code changes.

@maxgolov maxgolov requested review from a team and bogdandrutu June 4, 2021 21:39
@codecov
Copy link

codecov bot commented Jun 4, 2021

Codecov Report

Merging #825 (60c77e8) into main (93dd39b) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #825   +/-   ##
=======================================
  Coverage   95.52%   95.52%           
=======================================
  Files         156      156           
  Lines        6618     6618           
=======================================
  Hits         6321     6321           
  Misses        297      297           

@ThomsonTan
Copy link
Contributor

Is it more conservative to snap the latest tag instead of main branch?

@maxgolov
Copy link
Contributor Author

maxgolov commented Jun 5, 2021

Is it more conservative to snap the latest tag instead of main branch?

My understanding is it's generally both safer and more predictable to track to specific commit. And the usual flow is:

cd submodule
git pull
cd ..
git add submodule
git commit -m "Updated version to X"

This guarantees that by default when you perform recursive cloning, you get a set of deps, a baseline that is guaranteed to work / passes CI. It would've been a bit less predictable if we automated that process to automagically update whenever there's a new commit in the proto repo ( we could, but I don't think we should ).

@lalitb
Copy link
Member

lalitb commented Jun 5, 2021

Thanks for the update. Should we also update the proto package used by bazel as part of this PR:

strip_prefix = "opentelemetry-proto-0.6.0",

@lalitb
Copy link
Member

lalitb commented Jun 7, 2021

Merging now. For bazel, we can do it in separate PR.

@lalitb lalitb merged commit f88877c into main Jun 7, 2021
@lalitb lalitb deleted the maxgolov/opentelemetry-proto branch June 7, 2021 03:56
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.

4 participants