Closed shruti2522 closed 5 months ago
shruti2522
commented
6 months ago This approach is wrong and causing errors. Working on using GetFullSchemaType function to extract the referenced by info from the base information being returned by it. Would push the changes soon.
Reverting the changes for now.
coveralls
commented
6 months ago This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.
| Changes Missing Coverage | Covered Lines | Changed/Added Lines | % | ||
|---|---|---|---|---|---|
| pkg/tools/gen/gendoc.go | 21 | 24 | 87.5% | ||
| <!-- | Total: | 29 | 32 | 90.63% | --> |
| Files with Coverage Reduction | New Missed Lines | % | ||
|---|---|---|---|---|
| pkg/tools/override/override.go | 2 | 81.82% | ||
| pkg/tools/gen/genkcl_json.go | 2 | 73.33% | ||
| pkg/tools/gen/types.go | 4 | 88.99% | ||
| pkg/tools/gen/utils.go | 5 | 68.42% | ||
| pkg/tools/gen/genkcl_terraform.go | 7 | 79.44% | ||
| pkg/tools/gen/gengo.go | 7 | 85.42% | ||
| pkg/tools/gen/genkcl_proto.go | 9 | 83.5% | ||
| pkg/settings/utils_settings_yaml.go | 10 | 75.65% | ||
| pkg/tools/gen/genpb.go | 12 | 83.02% | ||
| pkg/tools/gen/genkcl_yaml.go | 14 | 81.18% | ||
| <!-- | Total: | 285 | --> |
| Totals | |
|---|---|
| Change from base Build 9253785111: | 1.4% |
| Covered Lines: | 3394 |
| Relevant Lines: | 5336 |
shruti2522
commented
6 months ago I have made the following changes:
GetFullSchemaType function in api.go to provide base schema information.genopenapi.go to get ReferencedBy info from base schema info being returned by GetFullschemaType.gendoc.go to populate the Referenced by info on render.Referenced By section to schemaDoc.gotmpl which is printed only when there are referenced schemas.Now, the only error while execution is the output of md-got, the Referenced By field is somehow not getting printed, maybe we need to update the testdata. Need some reviews here @Peefy .
Peefy
commented
6 months ago @shruti2522 Thank you, I believe there is no problem with the current overall implementation approach. I have left some details and methods for modifying test cases.
shruti2522
commented
6 months ago I guess there is some issue with the rendering logic too in gendoc.go, since the template is not able to render the Referenced By section. Working on the fix.
Peefy
commented
6 months ago I guess there is some issue with the rendering logic too in
gendoc.go, since the template is not able to render theReferenced Bysection. Working on the fix.
OK. But I think the current referencedBy logic may have been reversed firstly.
$ref Schema B, indicating that B's referencedBy contains A.shruti2522
commented
6 months ago I see. I'll review the logic and make sure that Schema A properly references Schema B in the referencedBy field. Will update accordingly once fixed.
Peefy
commented
5 months ago Hello @shruti2522, are you still working on this PR? Because we are preparing to release a new version of kcl-go in the next two days.
shruti2522
commented
5 months ago Hello @shruti2522, are you still working on this PR? Because we are preparing to release a new version of kcl-go in the next two days.
Yes @Peefy, I will push the changes by tomorrow at the latest.
1. Does this PR affect any open issues?(Y/N) and add issue references (e.g. "fix #123", "re #123".):
2. What is the scope of this PR (e.g. component or file name):
kcl-lang/kcl-go/pkg/tools/gen
3. Provide a description of the PR(e.g. more details, effects, motivations or doc link):
4. Are there any breaking changes?(Y/N) and describe the breaking changes(e.g. more details, motivations or doc link):
5. Are there test cases for these changes?(Y/N) select and add more details, references or doc links: