Skip to content

Connection string overrides #2471

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 17 commits into from
Aug 30, 2022
Merged

Connection string overrides #2471

merged 17 commits into from
Aug 30, 2022

Conversation

@trask
Copy link
Member

@trask trask commented Aug 27, 2022

Built on top of #2468

@lgtm-com
Copy link

lgtm-com bot commented Aug 27, 2022

This pull request fixes 10 alerts when merging c4a39c1 into 86cba45 - view on LGTM.com

fixed alerts:

  • 10 for Cross-site scripting

@trask trask changed the base branch from main to smoke-test-health-checks August 28, 2022 00:41
@lgtm-com
Copy link

lgtm-com bot commented Aug 28, 2022

This pull request fixes 11 alerts when merging fd19394 into 86cba45 - view on LGTM.com

fixed alerts:

  • 10 for Cross-site scripting
  • 1 for Potential output resource leak

@lgtm-com
Copy link

lgtm-com bot commented Aug 28, 2022

This pull request fixes 1 alert when merging fd19394 into c3853ce - view on LGTM.com

fixed alerts:

  • 1 for Potential output resource leak

@trask trask force-pushed the connection-string-overrides branch from fd19394 to e295faa Compare August 28, 2022 01:36
@lgtm-com
Copy link

lgtm-com bot commented Aug 28, 2022

This pull request fixes 1 alert when merging e295faa into c3853ce - view on LGTM.com

fixed alerts:

  • 1 for Potential output resource leak

@trask trask force-pushed the connection-string-overrides branch 2 times, most recently from d158b99 to 7dc43a8 Compare August 28, 2022 02:13
@lgtm-com
Copy link

lgtm-com bot commented Aug 28, 2022

This pull request fixes 1 alert when merging 7dc43a8 into c3853ce - view on LGTM.com

fixed alerts:

  • 1 for Potential output resource leak

@trask trask force-pushed the smoke-test-health-checks branch from c3853ce to e5d3b3f Compare August 29, 2022 00:34
@trask trask force-pushed the connection-string-overrides branch from 7dc43a8 to 0f7c497 Compare August 29, 2022 00:34
@trask trask mentioned this pull request Aug 29, 2022
Merged
@lgtm-com
Copy link

lgtm-com bot commented Aug 29, 2022

This pull request fixes 1 alert when merging 0f7c497 into e5d3b3f - view on LGTM.com

fixed alerts:

  • 1 for Potential output resource leak

@lgtm-com
Copy link

lgtm-com bot commented Aug 29, 2022

This pull request fixes 1 alert when merging 3573276 into e5d3b3f - view on LGTM.com

fixed alerts:

  • 1 for Potential output resource leak

@trask trask marked this pull request as ready for review August 29, 2022 03:45
@lgtm-com
Copy link

lgtm-com bot commented Aug 29, 2022

This pull request fixes 1 alert when merging f3a4205 into e5d3b3f - view on LGTM.com

fixed alerts:

  • 1 for Potential output resource leak

heyams
heyams reviewed Aug 29, 2022
View reviewed changes
@lgtm-com
Copy link

lgtm-com bot commented Aug 29, 2022

This pull request fixes 1 alert when merging 25afac6 into e5d3b3f - view on LGTM.com

fixed alerts:

  • 1 for Potential output resource leak

heyams
heyams reviewed Aug 29, 2022
View reviewed changes
@trask trask force-pushed the smoke-test-health-checks branch from e5d3b3f to 7deb2f4 Compare August 29, 2022 20:13
Base automatically changed from smoke-test-health-checks to main August 29, 2022 20:29
@trask trask force-pushed the connection-string-overrides branch from 25afac6 to a1767c3 Compare August 29, 2022 20:30
@lgtm-com
Copy link

lgtm-com bot commented Aug 29, 2022

This pull request fixes 1 alert when merging 1f090a8 into d7a916f - view on LGTM.com

fixed alerts:

  • 1 for Potential output resource leak

@lgtm-com
Copy link

lgtm-com bot commented Aug 29, 2022

This pull request fixes 1 alert when merging 344d7c7 into d7a916f - view on LGTM.com

fixed alerts:

  • 1 for Potential output resource leak

heyams
heyams approved these changes Aug 30, 2022
View reviewed changes
...ure/monitor/opentelemetry/exporter/implementation/configuration/ConnectionStringBuilder.java Outdated
// visible for testing
static final int CONNECTION_STRING_MAX_LENGTH = 4096;

private String parsedFrom;
Copy link
Contributor

@heyams heyams Aug 30, 2022

Choose a reason for hiding this comment

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

can we rename this to be consistent with ConnectionString?

Copy link
Member Author

@trask trask Aug 30, 2022

Choose a reason for hiding this comment

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

done

@trask trask merged commit f8137a7 into main Aug 30, 2022
@trask trask deleted the connection-string-overrides branch August 30, 2022 04:40
@lgtm-com
Copy link

lgtm-com bot commented Aug 30, 2022

This pull request fixes 1 alert when merging 56ed47c into d7a916f - view on LGTM.com

fixed alerts:

  • 1 for Potential output resource leak

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jeanbisutti jeanbisutti Awaiting requested review from jeanbisutti jeanbisutti is a code owner

1 more reviewer

@heyams heyams heyams approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@trask @heyams