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.

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

Utilizing the new TS support of open-ux-tools #58

Merged
merged 14 commits into from
Dec 14, 2022
Merged

Conversation

tobiasqueck
Copy link
Contributor

@tobiasqueck tobiasqueck commented Aug 29, 2022

The latest version of the @sap-ux/ui5-application-writer and the @sap-ux/fe-fpm-writer support the usage of Typescript based on the structure suggested by @petermuessig at https://github.com/ui5-community/ui5-ecosystem-showcase/tree/main/packages/ui5-app-ts.
This PR enables the functionality for easy-ui5.

Copy link
Member

@petermuessig petermuessig left a comment

Choose a reason for hiding this comment

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

Will the TS support completely replace the JS support? In this case the codeassist option maybe useless since it will always provide codeassist, right?

generators/app/index.js Outdated Show resolved Hide resolved
generators/enablefpm/index.js Outdated Show resolved Hide resolved
generators/newfpmpage/index.js Outdated Show resolved Hide resolved
Copy link
Member

@petermuessig petermuessig left a comment

Choose a reason for hiding this comment

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

Basically I'm fine with the change and we can go ahead - just some FYI comments...

generators/newfpmpage/index.js Outdated Show resolved Hide resolved
generators/app/index.js Show resolved Hide resolved
@petermuessig
Copy link
Member

@tobiasqueck I just had some FYI comments and let you check them - I'm fine with the change, so if you wanna go ahead with the change we can merge it.

@tobiasqueck
Copy link
Contributor Author

@marianfoo @vobu @nicoschoenteich may I get a review from one of you? @petermuessig reviewed and approved but I'd prefer a 2nd review.

@nicoschoenteich
Copy link
Collaborator

Hi @tobiasqueck, thanks for all the work. I would love to do a review, but unfortunately am incredibly busy with Devtoberfest prep. I will have a look, once things calm down, but can't promise anything.

Copy link
Contributor

@MariusFreitag MariusFreitag left a comment

Choose a reason for hiding this comment

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

Thank you very much, this now works fine for me and is a great addition!

Copy link
Collaborator

@nicoschoenteich nicoschoenteich left a comment

Choose a reason for hiding this comment

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

Veery late to the party... But I think this is a great addition and we can go ahead and merge. Thanks @tobiasqueck!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants