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

Fix to generate launch script for TS target and print informational message #2090

Merged
merged 7 commits into from
Nov 9, 2023
Merged

Fix to generate launch script for TS target and print informational message #2090

merged 7 commits into from
Nov 9, 2023

Conversation

hokeun
Copy link
Member

@hokeun hokeun commented Nov 8, 2023 •
edited
Loading

This fixes #1986.

Also did a minor code cleanup.

@hokeun hokeun linked an issue Nov 8, 2023 that may be closed by this pull request
lfc gives no clue how to run TypeScript code #1986
Closed
@hokeun hokeun requested review from lhstrh and edwardalee November 8, 2023 19:20
edwardalee
edwardalee approved these changes Nov 8, 2023
View reviewed changes
Copy link
Collaborator

@edwardalee edwardalee left a comment

Choose a reason for hiding this comment

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

LGTM.

lhstrh
lhstrh requested changes Nov 8, 2023
View reviewed changes
Copy link
Member

@lhstrh lhstrh left a comment •
edited
Loading

Choose a reason for hiding this comment

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

This looks great and it works well, but I have some suggestions for improvement:

  • don't just print the directory but print the full path of the executable
  • adjust the "Generating launcher for federation" message to do the same

@lhstrh lhstrh changed the title Create shell script for TS target and print message for the script. Generated launch script for TS target and informational message Nov 8, 2023
@hokeun
Copy link
Member Author

hokeun commented Nov 8, 2023

  • adjust the "Generating launcher for federation" message to do the same

Could you elaborate on this? I'm not sure what you meant by "do the same."

@hokeun hokeun changed the title Generated launch script for TS target and informational message Generate launch script for TS target and print informational message Nov 8, 2023
@hokeun hokeun changed the title Generate launch script for TS target and print informational message Fix to generate launch script for TS target and print informational message Nov 8, 2023
@lhstrh
Copy link
Member

lhstrh commented Nov 8, 2023

  • adjust the "Generating launcher for federation" message to do the same

Could you elaborate on this? I'm not sure what you meant by "do the same."

Sorry for not being clear. I just pushed a fix here because it was quicker than trying to clarify 😄

See b6eba5f

@lhstrh lhstrh enabled auto-merge November 8, 2023 22:34
@lhstrh lhstrh added this pull request to the merge queue Nov 9, 2023
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Nov 9, 2023
@lhstrh lhstrh added this pull request to the merge queue Nov 9, 2023
Merged via the queue into master with commit 6c3a1b8 Nov 9, 2023
@lhstrh lhstrh deleted the ts-bin branch November 9, 2023 03:42
@lhstrh lhstrh added the enhancement Enhancement of existing feature label Jan 23, 2024
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

@edwardalee edwardalee edwardalee approved these changes

Assignees
No one assigned
Labels
enhancement Enhancement of existing feature
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

lfc gives no clue how to run TypeScript code
3 participants
@hokeun @lhstrh @edwardalee