-
Notifications
You must be signed in to change notification settings - Fork 246
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
feat(spec): model submodules in the Assembly schema #1563
Conversation
Adds a `submodules` property on the `Assembly` structure, and carry it forward in the `dependencyClosure`, so the information can later be used to improve code generation for languages such as Python where the submodule structure is modeled as first-class entity that needs to be propertly dealt with. It can also help with properly adjusting the submodule names so they look more "native" in target languages, without facing problems when trying to generate type names in dependent packages. The forwarding of dependent submodules is not exercized in the current regression test suite because of a pair of other bugs (#1528, #1557) that need to be addressed before the generated Python code can successfully load. The last of those PRs to be merged will incldude the necessary test coverage.
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
* | ||
* @default none | ||
*/ | ||
submodules?: { [fqn: string]: SourceLocatable & Targetable }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where is the readme
contents going to be? Is it under Targetable
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could be added to Targetable
(moved there from Assembly
) yeah...
I would do this when I actually start processing READMEs in a subsequent PR. I don't want to bloat this one :)
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
Thank you for contributing! ❤️ I will now look into making sure the PR is up-to-date, then proceed to try and merge it! |
Merging (with squash)... |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
Merging (with squash)... |
Commit Message
feat(spec): model submodules in the Assembly schema (#1563)
Adds a
submodules
property on theAssembly
structure, and carry itforward in the
dependencyClosure
, so the information can later be usedto improve code generation for languages such as Python where the
submodule structure is modeled as first-class entity that needs to be
propertly dealt with. It can also help with properly adjusting the
submodule names so they look more "native" in target languages, without
facing problems when trying to generate type names in dependent packages.
The forwarding of dependent submodules is not exercized in the current
regression test suite because of a pair of other bugs (#1528, #1557)
that need to be addressed before the generated Python code can
successfully load. The last of those PRs to be merged will incldude the
necessary test coverage. This change is necessary for these PRs to
be able to fix their respective issues.
End Commit Message
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.