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

feat(alias-import): update runtime classes to understand import aliasing #860

Merged
merged 7 commits into from
Jul 11, 2024
Merged

feat(alias-import): update runtime classes to understand import aliasing #860

merged 7 commits into from
Jul 11, 2024

Conversation

salujajaskeerat
Copy link
Contributor

  • TODO marked
  • aliasImport handling in runtime separated from rest code to prevent breaking changes

Closes #859

Changes

Flags

Screenshots or Video

Related Issues

  • Issue #
  • Pull Request #

Author Checklist

  • Ensure you provide a DCO sign-off for your commits using the --signoff option of git commit.
  • Vital features and changes captured in unit and/or integration tests
  • Commits messages follow AP format
  • Extend the documentation, if necessary
  • Merging to main from fork:branchname

@salujajaskeerat salujajaskeerat force-pushed the sahib/i859/update-core-runtime-alias-import branch from 4d26b39 to 828ff2b Compare June 21, 2024 11:12
mttrbrts
mttrbrts reviewed Jun 25, 2024
View reviewed changes
Copy link
Member

@mttrbrts mttrbrts left a comment

Choose a reason for hiding this comment

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

There are a couple of linting failures in the build. I'm happy to approve once these and other existing comments are resolved.

@salujajaskeerat
Copy link
Contributor Author

There are a couple of linting failures in the build. I'm happy to approve once these and other existing comments are resolved.
@mttrbrts

The test would still fail because some depend on the new parser (cto), which has not yet been merged and is on a different branch. Locally, I have tested with the new parser; they work fine.
We should merge this PR after the cto-parser PR is mergerd

@salujajaskeerat salujajaskeerat mentioned this pull request Jun 25, 2024
Open
@mttrbrts
Copy link
Member

I see one about a missing JsDoc?

@ekarademir
Copy link
Contributor

Add a test case for extending an imported type to create a new class declaration.

@salujajaskeerat
Copy link
Contributor Author

I see one about a missing JsDoc?

It was regarding the documentation of function (comments ). It has been fixed now

@salujajaskeerat
Copy link
Contributor Author

Add a test case for extending an imported type to create a new class declaration.

Done

ekarademir
ekarademir approved these changes Jun 28, 2024
View reviewed changes
Copy link
Contributor

@ekarademir ekarademir left a comment

Choose a reason for hiding this comment

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

Pending upstream changes

@ekarademir ekarademir changed the title feat(alias import): Update runtime classes to understand import aliasing feat(alias-import): update runtime classes to understand import aliasing Jul 8, 2024
@salujajaskeerat
feat(alias import): Optional field aliasImport added to basemodelmanager
630d247 
- TODO marked
- aliasImport handling in runtime separated from rest code to prevent
  breaking changes

Signed-off-by: Jaskeerat Singh Saluja <[email protected]>
@salujajaskeerat
feat(alias import): modelfile.js alias imports handled
b48a375 
- alias types mapped to fqn in the importShortNames

Signed-off-by: Jaskeerat Singh Saluja <[email protected]>
@salujajaskeerat
feat(import alias):concerto-core modefile update
bd3926a 
- Handled PR changes
- types can't be aliased to primitive types

Signed-off-by: Jaskeerat Singh Saluja <[email protected]>
@salujajaskeerat
feat(alias import): Test Cases added
bb08d49 
Following are the test-cases added:

- resolve alias type from import
- validate alias type
- validate map using alias type as value

Signed-off-by: Jaskeerat Singh Saluja <[email protected]>
@salujajaskeerat
feat(alias import): Types updated ; Minor version updated
68b37ac 
Signed-off-by: Jaskeerat Singh Saluja <[email protected]>
@salujajaskeerat
feat(alias): Pr suggestion added
56356ca 
- Added new test case where a concept is extended on a aliased import
  type concept
- removed unecessary comments and formated code

Signed-off-by: Jaskeerat Singh Saluja <[email protected]>
@salujajaskeerat
feat(alias): aliasName renamed to aliasedName
e84cf14 
Signed-off-by: Jaskeerat Singh Saluja <[email protected]>
@salujajaskeerat salujajaskeerat force-pushed the sahib/i859/update-core-runtime-alias-import branch from cc0816f to e84cf14 Compare July 9, 2024 06:19
ekarademir
ekarademir approved these changes Jul 9, 2024
View reviewed changes
packages/concerto-core/package.json
@@ -1,6 +1,6 @@
{
"name": "@accordproject/concerto-core",
"version": "3.17.0",
"version": "3.17.1",
Copy link
Contributor

Choose a reason for hiding this comment

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

You can leave this change to the automation.

@ekarademir ekarademir merged commit 913b95d into accordproject:main Jul 11, 2024
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mttrbrts mttrbrts mttrbrts left review comments

@dselman dselman dselman approved these changes

@ekarademir ekarademir ekarademir approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Update runtime classes to understand import aliasing
4 participants
@salujajaskeerat @mttrbrts @ekarademir @dselman