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

Feature/11 synchronize the mccl code with the one included in the vts repo #12

Conversation

lmalenfant
Copy link
Member

Since we have not made any MCCL specific changes to the VTS since the last release, I was able to update the MCCL repo with the recent changes from the VTS repo. This PR has the same changes as the branch on the VTS repo so we should validate there first and then we can validate and merge this one. Creating this PR alongside the VTS one will allow us to perform the SonarCloud checks, these only run on the MonteCarlo repo.

I didn't want to wait until the next release to make these changes because we had some more complex changes (shared cs files) and they could have caused problems.

VirtualPhotonics/VTS#108

Lisa Malenfant added 2 commits September 30, 2023 16:17
…L application

Remove the UnitTestCoverage because it did not work for the code coverage
Remove the SonarCloudAnalysis file because this is handled in a action
@lmalenfant lmalenfant linked an issue Oct 1, 2023 that may be closed by this pull request
Copy link
Member

Choose a reason for hiding this comment

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

Line 26 puts out version "1.0". Should we edit this to match MCCL Program: GetVersionNumber(3)}\n"); ?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes we should! Nice catch! I'll update it on the VTS branch first and commit here also.

Copy link
Member

Choose a reason for hiding this comment

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

I am visually reviewing. What is best way for me to programmatically review MCCL and MCPP Program,Setup and tests?

Copy link
Member Author

Choose a reason for hiding this comment

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

We should review the changes on the VTS repo first:
VirtualPhotonics/VTS#108

Once we have reviewed on VTS, those changes will be the same here, you can just clone this repo locally and build and run the unit tests.

Copy link
Member

Choose a reason for hiding this comment

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

Oh, I see now I think. I can view these same changes on 107?

@sonarcloud
Copy link

sonarcloud bot commented Oct 1, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 2 Code Smells

65.2% 65.2% Coverage
0.0% 0.0% Duplication

Copy link
Member

@hayakawa16 hayakawa16 left a comment

Choose a reason for hiding this comment

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

If these changes match those in VTS#108, I approve.

@lmalenfant
Copy link
Member Author

If these changes match those in VTS#108, I approve.

They don't completely match, this branch contains all the changes since the last release.

@hayakawa16
Copy link
Member

I see. I've reviewed those additional changes on prior PRs though correct?

@lmalenfant
Copy link
Member Author

The csproj files are not the same, the VTS repo uses the VTS project and this repo uses the NuGet library so there are a few differences. It might be good to clone and make sure it runs as expected, we don't have a BuildTestRelease for this repo, we can discuss adding something for this repo.

@lmalenfant
Copy link
Member Author

I have merged the branch in the VTS repo but I will hold off on merging this one until after we meet so we can go over the changes together.

@hayakawa16
Copy link
Member

Makes sense on the csproj files. And yes we can discuss next Wednesday.

@lmalenfant lmalenfant merged commit 6bd295d into master Oct 4, 2023
5 checks passed
@lmalenfant lmalenfant deleted the feature/11-synchronize-the-mccl-code-with-the-one-included-in-the-vts-repo branch October 4, 2023 21:00
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.

Synchronize the MCCL code with the one included in the VTS repo
2 participants