Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Add instrument.sh script #1354
Add instrument.sh script #1354
Changes from 21 commits
730db12
79abbab
c01a215
4d69231
f68b1f3
1a25aed
5a8ad00
7b2f1b6
6328380
e2d3c68
0615c62
0f701a5
4d48926
39e6d95
559d4c2
b48fc9a
c169b7c
1d36a17
38508fe
3c2ee52
3e83664
b92f4b2
5fd1675
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You might consider breaking the "Instrument a .NET application" section into two smaller sections: 1) Environment variables and 2) Setup script. Based on my reading, that would help make sure someone who already sets the environment variables at the top doesn't try to run the instrument script when they don't have to
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I totally agree. Tomorrow, I will create a separate PR to address it 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this should point to the release artifact instead of to the actual source. Specially when main is not a release tree.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not sure as any
Writer
(approver) can change/remove/add release artifacts. Themain
branch is extremely protected - requires GPG signatures, also only the maintainers can merge/push to `main. We can consider changing it to a "release tag" during the release process. WDYT?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@RassK PTAL #1357