kontent-ai / model-generator-net

Kontent.ai .NET model generator.
https://www.nuget.org/packages/Kontent.Ai.ModelGenerator
MIT License
17 stars 19 forks source link

Feature/124 replace management client #128

Closed Sevitas closed 2 years ago

Sevitas commented 2 years ago

Motivation

Which issue does this fix? Fixes #124

Checklist

How to test

everything works as before

codecov[bot] commented 2 years ago

Codecov Report

Merging #128 (1072ec6) into vNext (393c83c) will increase coverage by 2.04%. The diff coverage is 76.33%.

Impacted file tree graph

@@            Coverage Diff             @@
##            vNext     #128      +/-   ##
==========================================
+ Coverage   84.29%   86.34%   +2.04%     
==========================================
  Files          21       23       +2     
  Lines         796      842      +46     
  Branches       81       78       -3     
==========================================
+ Hits          671      727      +56     
+ Misses        104       96       -8     
+ Partials       21       19       -2     
Impacted Files Coverage Δ
...o.Kontent.ModelGenerator.Core/CodeGeneratorBase.cs 44.15% <44.15%> (ø)
...ntent.ModelGenerator.Core/DeliveryCodeGenerator.cs 74.68% <74.68%> (ø)
...ent.ModelGenerator.Core/ManagementCodeGenerator.cs 80.00% <80.00%> (ø)
src/Kentico.Kontent.ModelGenerator/Program.cs 83.87% <90.90%> (+1.26%) :arrow_up:
...Generator.Core/Common/ClassCodeGeneratorFactory.cs 100.00% <100.00%> (ø)
...tent.ModelGenerator.Core/Common/ClassDefinition.cs 100.00% <100.00%> (ø)
...ico.Kontent.ModelGenerator.Core/Common/Property.cs 100.00% <100.00%> (ø)
...nerator.Core/Configuration/CodeGeneratorOptions.cs 100.00% <100.00%> (+6.66%) :arrow_up:
...Generators/Class/DeliveryClassCodeGeneratorBase.cs 100.00% <100.00%> (ø)
...delGenerator.Core/Helpers/DeliveryElementHelper.cs 100.00% <100.00%> (ø)
... and 5 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 393c83c...1072ec6. Read the comment docs.

Sevitas commented 2 years ago

Looks good. 👍

Nice touch rewriting content management to management 👍

Let's just sync with @Kentico/developer-experience team according to argument renaming.

I've also changed the console arg "-c" to "-m", representing whether to use MAPI or DAPI. What do you think about that? Isn't it a too much of a breaking change?

Simply007 commented 2 years ago

Looks good. 👍 Nice touch rewriting content management to management 👍 Let's just sync with @Kentico/developer-experience team according to argument renaming.

I've also changed the console arg "-c" to "-m", representing whether to use MAPI or DAPI. What do you think about that? Isn't it a too much of a breaking change?

Agree. We already have breaking changes in arguments names. It is not a big of a change for customers.

Sevitas commented 2 years ago

Let's discuss following:

Better naming for test would be suitable for ManagementCodeGeneratorTests

Integration test IntegrationTest is checking > 10 instead of exact number

ValidationExtensions.Validate

Compilability of the Management class would be suitable as it is in IntegrationTest_GeneratedCodeCompilesWithoutErrors using CSharpCompilation.

This might be added in separate issues:

Missing element types in tests/Kentico.Kontent.ModelGeneratorTests/Fixtures/*.json:

  • DAPI: Custom element, Subpages element
  • MAPI: Guidelines, Custom, Subpages

Run the main program with various configurations and mocked generators to check correct generators are being called.

Missing elements were fixed in the #136.