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

TypeScript generator cleanups #1457

Merged
merged 5 commits into from
Nov 8, 2022
Merged

TypeScript generator cleanups #1457

merged 5 commits into from
Nov 8, 2022

Conversation

oowekyala
Copy link
Collaborator

Changes extracted from #1441

ExpressionGenerator is removed because it is redundant to TargetTypes. The rest should be a simple refactoring without functional change.

@oowekyala
Implement missing method in TsTypes
98db015 
Fixed-size lists are now explicitly disallowed, previously
they would have generated malformed code.
@oowekyala
Copy link
Collaborator Author

oowekyala commented Nov 4, 2022 •
edited
Loading

The remaining failing tests are because of #1455, and there is also an LSP test failing randomly (not related to this change): https://github.com/lf-lang/lingua-franca/actions/runs/3386947686/jobs/5639171134#step:16:420

I don't understand why an error is expected here. Is the language server checking for undefined names in a Python file? How accurate can that be?

@petervdonovan this looks like the same failure as in #1456: https://github.com/lf-lang/lingua-franca/actions/runs/3404064946/jobs/5661031451#step:16:494
and #1451 https://github.com/lf-lang/lingua-franca/actions/runs/3406562994/jobs/5667063554
Do you have an idea, how we could fix that?

@oowekyala oowekyala marked this pull request as ready for review November 4, 2022 13:12
@oowekyala oowekyala requested review from cmnrd and lhstrh November 7, 2022 10:45
@oowekyala oowekyala mentioned this pull request Nov 7, 2022
Merged
@cmnrd
Copy link
Collaborator

cmnrd commented Nov 7, 2022

After running the workflow again, the LSP error also shows in master: https://github.com/lf-lang/lingua-franca/actions/runs/3384118963/jobs/5673489810
I suspect that in the last run the LSP test war run with a old version of the MacOS runner.

@petervdonovan petervdonovan mentioned this pull request Nov 7, 2022
Merged
lhstrh
lhstrh approved these changes Nov 8, 2022
View reviewed changes
Copy link
Member

@lhstrh lhstrh left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks for these cleanups, @oowekyala 👍

org.lflang/src/org/lflang/CommonExtensions.kt Show resolved Hide resolved
org.lflang/src/org/lflang/generator/GeneratorUtils.kt Show resolved Hide resolved
org.lflang/src/org/lflang/generator/ts/TSImportPreambleGenerator.kt Outdated Show resolved Hide resolved
@lhstrh lhstrh changed the title Cleanup TS generator TypeScript generator cleanups Nov 8, 2022
@lhstrh lhstrh added the refactoring Code quality enhancement label Nov 8, 2022
lhstrh
lhstrh approved these changes Nov 8, 2022
View reviewed changes
org.lflang/src/org/lflang/generator/GeneratorUtils.kt Outdated Show resolved Hide resolved
org.lflang/src/org/lflang/generator/GeneratorUtils.kt Outdated Show resolved Hide resolved
org.lflang/src/org/lflang/generator/GeneratorUtils.kt Outdated Show resolved Hide resolved
@lhstrh lhstrh merged commit cd46166 into master Nov 8, 2022
@lhstrh lhstrh deleted the ts-cleanups branch November 8, 2022 19:35
oowekyala added a commit that referenced this pull request Nov 9, 2022
@oowekyala
Cleanup after #1457 merge
71ef8d0
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lhstrh lhstrh lhstrh approved these changes

@cmnrd cmnrd Awaiting requested review from cmnrd

Assignees
No one assigned
Labels
refactoring Code quality enhancement
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@oowekyala @cmnrd @lhstrh