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

[ci visibility] Fix copy-paste error in teamcity env vars #2562

Merged
merged 2 commits into from
Jan 18, 2023

Conversation

albertvaka
Copy link
Contributor

What does this PR do?
Fixes a copy-paste error where the wrong env var was being used to compute the pipeline URL for TeamCity.

@albertvaka albertvaka requested a review from a team January 16, 2023 17:20
@github-actions github-actions bot added the ci-app CI product for test suite instrumentation label Jan 16, 2023
Copy link
Member

@ivoanjo ivoanjo left a comment

Choose a reason for hiding this comment

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

👍 LGTM. Is this checked in system-tests or elsewhere? It looks like it'd be good to have test coverage for it :)

@ivoanjo
Copy link
Member

ivoanjo commented Jan 17, 2023

P.s.: We're working on fixing the unrelated CI hiccup, we should have it back to green soon and then I'll merge in the PR :)

@albertvaka
Copy link
Contributor Author

albertvaka commented Jan 17, 2023

We have a separate repo (not linking it here because it is private) with tests that run against all tracers (since the behaviour should be the same for all). We don't have tests for TeamCity yet but I will add them today.

@codecov-commenter
Copy link

Codecov Report

Merging #2562 (195d336) into master (04fd1be) will decrease coverage by 0.01%.
The diff coverage is 97.14%.

@@            Coverage Diff             @@
##           master    #2562      +/-   ##
==========================================
- Coverage   98.03%   98.02%   -0.01%     
==========================================
  Files        1124     1124              
  Lines       60656    60683      +27     
  Branches     2767     2767              
==========================================
+ Hits        59463    59487      +24     
- Misses       1193     1196       +3     
Impacted Files Coverage Δ
lib/datadog/ci/ext/environment.rb 98.41% <0.00%> (ø)
...iling_native_extension/native_extension_helpers.rb 96.73% <100.00%> (+0.53%) ⬆️
...filing/collectors/cpu_and_wall_time_worker_spec.rb 94.75% <100.00%> (+0.01%) ⬆️
...datadog/profiling/native_extension_helpers_spec.rb 97.69% <100.00%> (+0.27%) ⬆️
lib/datadog/core/diagnostics/environment_logger.rb 97.63% <0.00%> (-0.79%) ⬇️
spec/datadog/profiling/native_extension_spec.rb 98.11% <0.00%> (-0.63%) ⬇️
...dog/profiling/collectors/cpu_and_wall_time_spec.rb 97.48% <0.00%> (-0.42%) ⬇️
...ng/contrib/active_support/cache/instrumentation.rb 86.20% <0.00%> (-0.10%) ⬇️
...ec/datadog/tracing/contrib/sidekiq/patcher_spec.rb 100.00% <0.00%> (+4.00%) ⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@ivoanjo
Copy link
Member

ivoanjo commented Jan 18, 2023

I've merged in the master branch to fix CI, making this good to go -- pressing the magic button now!

@ivoanjo ivoanjo merged commit 984d026 into master Jan 18, 2023
@ivoanjo ivoanjo deleted the albertvaka/teamcity-fix-pipeline-url-env-var branch January 18, 2023 10:04
@github-actions github-actions bot added this to the 1.9.0 milestone Jan 18, 2023
@TonyCTHsu TonyCTHsu mentioned this pull request Jan 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci-app CI product for test suite instrumentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants