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

Usage of a published version of reactor-ts #1426

Merged
merged 23 commits into from
Oct 28, 2022
Merged

Usage of a published version of reactor-ts #1426

merged 23 commits into from
Oct 28, 2022

Conversation

petervdonovan
Copy link
Collaborator

@petervdonovan petervdonovan commented Oct 22, 2022 •
edited by lhstrh
Loading

Fixes #1359
Tasks completed:

  • Remove reactor-ts submodule
  • Facilitate local testing using a command line ./gradlew test --tests org.lflang.tests.runtime.TypeScriptTest.* -Druntime="file:<some-path-to-a-local-reactor-ts>"
  • Setup reactor-ts CI to run integration tests against a particular commit
  • Provide documentation about new development workflow

@petervdonovan
Test to see if CI is faster using published reactor-ts.
67333c1 
Note that we still use the types from the runtime version that is packed in the jar.
@petervdonovan
[build] Do not copy TS runtime from classpath.
af781d3
@lhstrh lhstrh changed the title Test to see if CI is faster using published reactor-ts. Use a published version of reactor-ts Oct 26, 2022
@lhstrh lhstrh marked this pull request as ready for review October 26, 2022 05:50
petervdonovan
petervdonovan commented Oct 26, 2022
View reviewed changes
Copy link
Collaborator Author

@petervdonovan petervdonovan left a comment

Choose a reason for hiding this comment

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

I can't approve this because of how GitHub behaves, but it looks right to me. I agree that it is cleaner to introduce the BuildParm enum.

org.lflang.cli/src/org/lflang/cli/Lfc.java Outdated Show resolved Hide resolved
@lhstrh lhstrh requested a review from cmnrd October 26, 2022 05:51
lhstrh and others added 2 commits October 25, 2022 22:51
@lhstrh @petervdonovan
Update org.lflang.cli/src/org/lflang/cli/Lfc.java
f77e727 
Co-authored-by: Peter Donovan <33707478+petervdonovan@users.noreply.github.com>
@lhstrh
Bring back cache and use version 3
4caadd8
cmnrd
cmnrd approved these changes Oct 26, 2022
View reviewed changes
Copy link
Collaborator

@cmnrd cmnrd left a comment

Choose a reason for hiding this comment

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

This looks good to me! The improvements for Linux and Windows are significant, but oddly on MacOS the tests are still running after 2 hours. Please also consider my comment regarding --runtime-version

test/TypeScript/README.md Show resolved Hide resolved
@lhstrh lhstrh force-pushed the ts-use-published-runtime branch from 69a3166 to 11e029a Compare October 27, 2022 07:51
@lhstrh lhstrh changed the title Use a published version of reactor-ts Usage of a published version of reactor-ts Oct 28, 2022
@lhstrh lhstrh merged commit b9c3cfa into master Oct 28, 2022
@lhstrh lhstrh deleted the ts-use-published-runtime branch October 28, 2022 04:44
@lhstrh lhstrh added the refactoring Code quality enhancement label Jan 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cmnrd cmnrd cmnrd approved these changes

@lhstrh lhstrh lhstrh approved these changes

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.

TypeScript tests are way too slow in CI
3 participants
@petervdonovan @cmnrd @lhstrh