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: new codegen annotations #230

Merged
merged 2 commits into from
Jan 7, 2022
Merged

feat: new codegen annotations #230

merged 2 commits into from
Jan 7, 2022

Conversation

octonato
Copy link
Member

@octonato octonato commented Jan 4, 2022 •
edited
Loading

No description provided.

@github-actions github-actions bot added kalix-runtime Runtime and SDKs sub-team javascript-sdk labels Jan 4, 2022
Base automatically changed from octonato/upgrade-to-proxy-0.8.5 to main January 5, 2022 13:02
@octonato octonato force-pushed the octonato/new-codegen-annotations branch from 380335f to 9070e37 Compare January 5, 2022 15:41
@octonato octonato force-pushed the octonato/new-codegen-annotations branch from 9070e37 to 3f9f77d Compare January 5, 2022 19:17
@octonato octonato marked this pull request as ready for review January 5, 2022 19:17
@octonato
Copy link
Member Author

octonato commented Jan 5, 2022

Ready for review.

We are now capable of building the intermediate AST, but we still need to take into consideration the case for names Actions and Views. I plan to do it on follow-up PR. We will need to add more tests.

We can also decide to reduce the scope for now. What we have now has feature parity with the previous annotation and therefore we can decide to leave named Actions and Views and unnamed Entities out.

@octonato octonato mentioned this pull request Jan 5, 2022
Open
johanandren
johanandren approved these changes Jan 7, 2022
View reviewed changes
Copy link
Contributor

@johanandren johanandren left a comment

Choose a reason for hiding this comment

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

LGTM, but weird to depend on local fork of Java SDK just to get access to proxy protocol/compile descriptors.

codegen/core/src/test/resources/test-files/codegen-annotation/generate-from-samples.sh Outdated Show resolved Hide resolved
@octonato octonato merged commit e869197 into main Jan 7, 2022
@octonato octonato deleted the octonato/new-codegen-annotations branch January 7, 2022 12:37
@octonato octonato mentioned this pull request Jan 7, 2022
Open
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@johanandren johanandren johanandren approved these changes

Assignees
No one assigned
Labels
javascript-sdk kalix-runtime Runtime and SDKs sub-team
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@octonato @johanandren