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

Modelcompiler updates #1284

Merged
merged 18 commits into from
Mar 29, 2021
Merged

Modelcompiler updates #1284

merged 18 commits into from
Mar 29, 2021

Conversation

opcfoundation-org
Copy link
Contributor

No description provided.

@CLAassistant
Copy link

CLAassistant commented Feb 13, 2021

CLA assistant check
All committers have signed the CLA.

@lgtm-com
Copy link

lgtm-com bot commented Feb 13, 2021

This pull request introduces 2 alerts and fixes 2 when merging d4642f1 into 4180fd8 - view on LGTM.com

new alerts:

  • 2 for Missing Dispose call on local IDisposable

fixed alerts:

  • 2 for Useless assignment to local variable

@lgtm-com
Copy link

lgtm-com bot commented Feb 13, 2021

This pull request introduces 2 alerts and fixes 2 when merging 90d7cec into 4180fd8 - view on LGTM.com

new alerts:

  • 2 for Missing Dispose call on local IDisposable

fixed alerts:

  • 2 for Useless assignment to local variable

@lgtm-com
Copy link

lgtm-com bot commented Feb 13, 2021

This pull request introduces 2 alerts and fixes 2 when merging 8f7cd8a into 4180fd8 - view on LGTM.com

new alerts:

  • 2 for Missing Dispose call on local IDisposable

fixed alerts:

  • 2 for Useless assignment to local variable

@opcfoundation-org
Copy link
Contributor Author

Martin, the diffs show code I did not change.
These diffs need to be discussed to make sure the merge happened correctly.

@AlinMoldovean
Copy link
Contributor

@opcfoundation-org , @mregen
Branch modelcompiler-updates has a merge commit with changes from branch prototyping_noservicemodel.
Those changes are already included in PR #1255.
This means we apply changes from both branches in this PR and later close #1255?

@mregen
Copy link
Contributor

mregen commented Feb 15, 2021

The PR is ok, but I cannot reproduce where the model compiler output comes from,
@opcfoundation-org are the generated files from uncommitted code?
Once we have a matching version to make this reproducible I think we can merge and delete the other PR.

@opcfoundation-org
Copy link
Contributor Author

Yes, chicken and egg problem. I needed to make sure the new generated works before I check in.
How can we produce a NuGet with the branch and verify that the sample apps run?

@mregen mregen added this to the 1.4.366 milestone Feb 18, 2021
@mregen
Copy link
Contributor

mregen commented Feb 18, 2021

lets finish all 1.4.365 PR/fixes before merging it to master, as this change will require more testing and should be bumped to 1.4.366

@opcfoundation-org
Copy link
Contributor Author

This can't be merged as is.
I need to produce a version that only has 1.04.9 errata.

@mregen
Copy link
Contributor

mregen commented Feb 18, 2021

ok, then I downgrade to draft until 1.04.9 errata is ready

@mregen mregen marked this pull request as draft February 18, 2021 11:06
@codecov
Copy link

codecov bot commented Feb 26, 2021

Codecov Report

❗ No coverage uploaded for pull request base (master@426dcba). Click here to learn what that means.
The diff coverage is 16.27%.

❗ Current head b3bc161 differs from pull request most recent head f7c95ae. Consider uploading reports for the commit f7c95ae to get more accurate results
Impacted file tree graph

@@            Coverage Diff            @@
##             master    #1284   +/-   ##
=========================================
  Coverage          ?   42.16%           
=========================================
  Files             ?      291           
  Lines             ?   105089           
  Branches          ?        0           
=========================================
  Hits              ?    44311           
  Misses            ?    60778           
  Partials          ?        0           
Impacted Files Coverage Δ
Libraries/Opc.Ua.Client/Session.cs 23.73% <0.00%> (ø)
...re/Stack/Configuration/ApplicationConfiguration.cs 42.55% <0.00%> (ø)
Stack/Opc.Ua.Core/Types/BuiltIn/NodeId.cs 48.82% <ø> (ø)
...pc.Ua.Server/Diagnostics/DiagnosticsNodeManager.cs 49.08% <13.88%> (ø)
Stack/Opc.Ua.Core/Types/BuiltIn/DataValue.cs 39.55% <100.00%> (ø)
Stack/Opc.Ua.Core/Types/Utils/RelativePath.cs 39.62% <100.00%> (ø)

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 426dcba...f7c95ae. Read the comment docs.

@mregen mregen marked this pull request as ready for review March 25, 2021 16:37
@mregen mregen marked this pull request as draft March 25, 2021 16:37
@lgtm-com
Copy link

lgtm-com bot commented Mar 25, 2021

This pull request introduces 2 alerts and fixes 2 when merging 298ca93 into b3bc161 - view on LGTM.com

new alerts:

  • 2 for Missing Dispose call on local IDisposable

fixed alerts:

  • 2 for Useless assignment to local variable

@lgtm-com
Copy link

lgtm-com bot commented Mar 26, 2021

This pull request introduces 2 alerts and fixes 2 when merging f7c95ae into b3bc161 - view on LGTM.com

new alerts:

  • 2 for Missing Dispose call on local IDisposable

fixed alerts:

  • 2 for Useless assignment to local variable

@mregen mregen marked this pull request as ready for review March 26, 2021 20:41
Copy link
Contributor

@mregen mregen left a comment

Choose a reason for hiding this comment

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

Ready for merge once all 365 PRs are done and merged in the release branch.

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.

None yet

5 participants