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

Application config improvements for cert recreation #1525

Merged
merged 9 commits into from
Sep 28, 2021
Merged

Conversation

mregen
Copy link
Contributor

@mregen mregen commented Sep 26, 2021

@lgtm-com
Copy link

lgtm-com bot commented Sep 26, 2021

This pull request introduces 2 alerts when merging d137a84 into 33ae8bb - view on LGTM.com

new alerts:

  • 1 for Constant condition
  • 1 for Missing Dispose call on local IDisposable

@lgtm-com
Copy link

lgtm-com bot commented Sep 27, 2021

This pull request introduces 2 alerts when merging ede6459 into 33ae8bb - view on LGTM.com

new alerts:

  • 1 for Constant condition
  • 1 for Missing Dispose call on local IDisposable

@lgtm-com
Copy link

lgtm-com bot commented Sep 27, 2021

This pull request introduces 4 alerts when merging d4765b1 into 33ae8bb - view on LGTM.com

new alerts:

  • 2 for Missing Dispose call on local IDisposable
  • 1 for Constant condition
  • 1 for Poor error handling: empty catch block

message.AppendLine("Loading a certificate with private key from the directory store.");
message.AppendLine("Ensure to call LoadPrivateKeyEx with password provider before calling Find(true).");
Utils.Trace(Utils.TraceMasks.Error, message.ToString());
Debug.Assert(!needPrivateKey, message.ToString());
Copy link
Contributor Author

Choose a reason for hiding this comment

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

remove debug assert...

@codecov
Copy link

codecov bot commented Sep 27, 2021

Codecov Report

Merging #1525 (531f112) into master (33ae8bb) will increase coverage by 0.03%.
The diff coverage is 88.70%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1525      +/-   ##
==========================================
+ Coverage   52.96%   52.99%   +0.03%     
==========================================
  Files         308      317       +9     
  Lines       58395    60329    +1934     
==========================================
+ Hits        30928    31974    +1046     
- Misses      27467    28355     +888     
Impacted Files Coverage Δ
...ificates/X509Certificate/CertificateBuilderBase.cs 88.23% <ø> (+1.17%) ⬆️
...a.Core/Security/Certificates/CertificateFactory.cs 86.15% <40.00%> (-1.15%) ⬇️
...ore/Security/Certificates/CertificateIdentifier.cs 25.57% <50.00%> (-0.48%) ⬇️
...a.Configuration/ApplicationConfigurationBuilder.cs 66.55% <100.00%> (-1.10%) ⬇️
...raries/Opc.Ua.Configuration/ApplicationInstance.cs 60.80% <100.00%> (+2.19%) ⬆️
...ecurity/Certificates/CertificateStoreIdentifier.cs 73.52% <100.00%> (ø)
...Core/Security/Certificates/CertificateValidator.cs 78.06% <100.00%> (+0.60%) ⬆️
Libraries/Opc.Ua.PubSub/PublishedData/DataSet.cs 16.66% <0.00%> (-83.34%) ⬇️
...raries/Opc.Ua.PubSub/Transport/UdpClientCreator.cs 12.23% <0.00%> (-6.25%) ⬇️
...aries/Opc.Ua.PubSub/Encoding/UadpDataSetMessage.cs 74.94% <0.00%> (-5.30%) ⬇️
... and 30 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 33ae8bb...531f112. Read the comment docs.

@lgtm-com
Copy link

lgtm-com bot commented Sep 28, 2021

This pull request introduces 4 alerts when merging ea5cd21 into 33ae8bb - view on LGTM.com

new alerts:

  • 2 for Missing Dispose call on local IDisposable
  • 1 for Constant condition
  • 1 for Poor error handling: empty catch block

Copy link
Contributor

@cristipogacean cristipogacean left a comment

Choose a reason for hiding this comment

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

LGTM

@lgtm-com
Copy link

lgtm-com bot commented Sep 28, 2021

This pull request introduces 2 alerts when merging 531f112 into 75744e2 - view on LGTM.com

new alerts:

  • 1 for Constant condition
  • 1 for Poor error handling: empty catch block

@mregen mregen merged commit e5baad9 into master Sep 28, 2021
@mregen mregen deleted the checkcert branch September 28, 2021 11:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants