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.

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

Upgrade/Replace to DTDL V3 enabled Parser #30

Merged
merged 9 commits into from
Aug 24, 2023
Merged

Upgrade/Replace to DTDL V3 enabled Parser #30

merged 9 commits into from
Aug 24, 2023

Conversation

sai-prajeet
Copy link
Collaborator

@sai-prajeet sai-prajeet commented Aug 9, 2023 •
edited
Loading

This PR contains the below changes:

  1. Added a new DTDL Parser Package version 1.0.52.
  2. Removed the existing Parser Package (Microsoft.Azure.DigitalTwins.Parser)
  3. Updated the System.Text.Json to latest stable version (7.0.3).
  4. Created a new test project Generator.V3.Tests that contains DTDL models with version 3.
  5. Renamed existing unit tests project to Generator.V2.Tests.
  6. The Access level for the properties in the file CloudToDeviceMethodOptions.cs under Generator.V2.Tests.Generated Project is changed to internal from public on running the unit tests as the original file under custom folder (Generator.Template Project) is having the properties with access level internal.

@sai-prajeet sai-prajeet requested a review from a team as a code owner August 9, 2023 02:04
tekgiant
tekgiant suggested changes Aug 9, 2023
View reviewed changes
src/Generator/Helper.cs Outdated Show resolved Hide resolved
@amdawood
Copy link

amdawood commented Aug 9, 2023

@sai-prajeet, please add tests that generate v3 models

amdawood
amdawood reviewed Aug 9, 2023
View reviewed changes
src/Generator/Helper.cs Outdated Show resolved Hide resolved
@sai-prajeet
Copy link
Collaborator Author

@microsoft-github-policy-service agree company="Microsoft"

@sai-prajeet
Copy link
Collaborator Author

@sai-prajeet, please add tests that generate v3 models
updated the test models context version to 3 and all the test cases are passing

amdawood
amdawood approved these changes Aug 10, 2023
View reviewed changes
Copy link

@amdawood amdawood left a comment

Choose a reason for hiding this comment

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

Please ensure the description is updated to reflect all the changes

@sai-prajeet
Copy link
Collaborator Author

Please ensure the description is updated to reflect all the changes

Thanks Amr. Updated the PR description

tekgiant
tekgiant suggested changes Aug 10, 2023
View reviewed changes
Copy link

@tekgiant tekgiant left a comment

Choose a reason for hiding this comment

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

This PR should be renamed to "Upgrade/Replace to DTDL V3 enabled Parser"

test/Generator.Tests/TestDtdlModels/Amenities.json Outdated Show resolved Hide resolved
@ms-mikeb ms-mikeb added type:feature Feature (new functionality) version:minor labels Aug 10, 2023
@sai-prajeet sai-prajeet changed the title added DTDLParser Package and removed existing Parser Package Upgrade/Replace to DTDL V3 enabled Parser Aug 10, 2023
ms-mikeb
ms-mikeb previously approved these changes Aug 23, 2023
View reviewed changes
@Shuvajyoti
Copy link
Collaborator

Please fix the build.

@sai-prajeet
Copy link
Collaborator Author

Please fix the build.

Fixed the Build issue

@ms-mikeb ms-mikeb merged commit 513fa87 into microsoft:main Aug 24, 2023
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tekgiant tekgiant tekgiant requested changes

@amdawood amdawood amdawood approved these changes

@ms-mikeb ms-mikeb ms-mikeb approved these changes

@Shuvajyoti Shuvajyoti Shuvajyoti approved these changes

@apoorvg apoorvg apoorvg approved these changes

@sagunase sagunase sagunase approved these changes

Assignees
No one assigned
Labels
type:feature Feature (new functionality) version:minor
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@sai-prajeet @amdawood @Shuvajyoti @tekgiant @sagunase @apoorvg @ms-mikeb