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

Explain the logical reasoning behind local Abseil snapshot + namespace renaming #797

Closed
wants to merge 10 commits into from

Conversation

maxgolov
Copy link
Contributor

No code changes.

This PR is a supplementary doc that explains the logical reasoning / merits of the process used to create / refresh the Abseil snapshot in PR #572

Since some changes were not trivial, a separate section is added to elaborate on these changes.

Hopefully we should not employ this (semi-manual) process too often.

@maxgolov maxgolov requested a review from a team May 25, 2021 21:46
@codecov
Copy link

codecov bot commented May 25, 2021

Codecov Report

Merging #797 (327c006) into main (703a2a5) will decrease coverage by 1.85%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #797      +/-   ##
==========================================
- Coverage   95.48%   93.64%   -1.84%     
==========================================
  Files         156      190      +34     
  Lines        6614     8953    +2339     
==========================================
+ Hits         6315     8383    +2068     
- Misses        299      570     +271     
Impacted Files Coverage Δ
...pentelemetry/context/propagation/noop_propagator.h 60.00% <0.00%> (-40.00%) ⬇️
sdk/src/trace/span.cc 84.50% <0.00%> (-6.96%) ⬇️
api/test/trace/provider_test.cc 73.34% <0.00%> (-6.66%) ⬇️
...lemetry/exporters/memory/in_memory_span_exporter.h 94.29% <0.00%> (-5.71%) ⬇️
...de/opentelemetry/trace/propagation/b3_propagator.h 94.03% <0.00%> (-2.46%) ⬇️
...ude/opentelemetry/common/key_value_iterable_view.h 86.67% <0.00%> (-2.22%) ⬇️
...lude/opentelemetry/sdk/trace/samplers/always_off.h 77.78% <0.00%> (-2.22%) ⬇️
sdk/test/trace/always_off_sampler_test.cc 84.62% <0.00%> (-2.05%) ⬇️
sdk/test/trace/always_on_sampler_test.cc 89.75% <0.00%> (-1.92%) ⬇️
...i/include/opentelemetry/trace/propagation/jaeger.h 96.50% <0.00%> (-1.06%) ⬇️
... and 85 more

@maxgolov
Copy link
Contributor Author

We seriously have to do something about codecov/project. It is:

  • rarely adds much (or any) value
  • noisy
  • creates an impression of CI failure when, in fact, CI is completely clean.

We may need to edit the associated GitHub action to avoid triggering red marks, and only treat it as optional, always pass showing the code coverage rate changes. Maybe there's an option to configure a min threshold that trips the failure, excluding anything that is under 1%.

Also, perhaps, if the list of changed files covers only Markdown or in /docs folder, we should avoid triggering any builds at all. That'd save environment and electrons that are unnecessarily disturbed.

@github-actions github-actions bot added the Stale label Nov 11, 2021
@github-actions github-actions bot closed this Nov 19, 2021
@lalitb
Copy link
Member

lalitb commented Nov 19, 2021

incorrectly closed by bot.

@lalitb lalitb reopened this Nov 19, 2021
@github-actions github-actions bot removed the Stale label Nov 20, 2021
@github-actions github-actions bot added the Stale label Jan 20, 2022
@github-actions github-actions bot closed this Jan 28, 2022
@lalitb lalitb reopened this Jan 28, 2022
@lalitb lalitb added do-not-stale and removed Stale labels Jan 28, 2022
@reyang reyang closed this Mar 6, 2022
@reyang reyang deleted the maxgolov/abseil_upgrade_doc branch March 6, 2022 20:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants