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

Change INSTALL_DIR to OTEL_DOTNET_AUTO_HOME in scripts #1375

Merged
merged 1 commit into from
Oct 6, 2022

Conversation

pellared
Copy link
Member

@pellared pellared commented Oct 6, 2022

Why

Part of #1373

  1. User can define a global machine-wide (or user-wide) OTEL_DOTNET_AUTO_HOME so that INSTALL_DIR (which is not a unique name) does not have to be provided when a custom installation path is used
    1. At some point install.sh could even set OTEL_DOTNET_AUTO_HOME as a global env var to facilitate using ./instrument.sh. Not now for sure - personally I want to postpone setting global env vars until we see that it is beneficial)
  2. Takes advantage of existing env var name which is required anyway

Also, change the default so that, by default, the ./instrument.sh could be used from a different location.

What

  • Change INSTALL_DIR env var script param to OTEL_DOTNET_AUTO_HOME
  • Change the default installation path to $HOME/.otel-auto-dotnet
    • Based on what dotnet-install.sh
    • It is not dependent on $PWD
    • Does not require admin rights
  • Remove RELEASES_URL script param (YAGNI)

Tests

CI

Checklist

  • CHANGELOG.md is updated.
  • Documentation is updated.
  • New features are covered by tests.

@github-actions github-actions bot requested a review from theletterf October 6, 2022 11:34
@pellared pellared marked this pull request as ready for review October 6, 2022 11:39
@pellared pellared requested a review from a team October 6, 2022 11:39
@pellared pellared changed the title Rename INSTALL_DIR to OTEL_DOTNET_AUTO_HOME in scripts Change INSTALL_DIR to OTEL_DOTNET_AUTO_HOME in scripts Oct 6, 2022
Copy link
Contributor

@rajkumar-rangaraj rajkumar-rangaraj left a comment

Choose a reason for hiding this comment

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

LGTM

@nrcventura nrcventura merged commit c395502 into open-telemetry:main Oct 6, 2022
@pellared pellared deleted the refine-install-dir branch October 6, 2022 21:20
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.

5 participants