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

[#1113][#1224] improvement(UI): TS support and improve global request handing #1308

Merged
merged 9 commits into from
Jan 4, 2024

Conversation

ch3yne
Copy link
Contributor

@ch3yne ch3yne commented Jan 3, 2024

What changes were proposed in this pull request?

  1. Support TypeScript
  2. Improve global reauest handing and add error messages

Why are the changes needed?

Fix: #1113
Fix: #1224
Fix: #1225

Does this PR introduce any user-facing change?

N/A

How was this patch tested?

N/A

@ch3yne ch3yne requested review from jerryshao and xunliu January 3, 2024 03:36
@ch3yne ch3yne self-assigned this Jan 3, 2024
@justinmclean
Copy link
Member

There is no need to add web/licenses/typescript.txt as we already have the ALv2 text in full.

@justinmclean
Copy link
Member

You also need to add the product that is "MIT License Copyright (c) 2020-present, Vben" to LICENSE.

web/LICENSE.bin Outdated

This product bundles various third-party components also under the
MIT license.

MIT License Copyright (c) 2020-present, Vben
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@justinmclean Is this the correct format for adding it?

Copy link
Member

Choose a reason for hiding this comment

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

you need to list the product name not the license and copyright

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Retained only Vben

@ch3yne
Copy link
Contributor Author

ch3yne commented Jan 3, 2024

There is no need to add web/licenses/typescript.txt as we already have the ALv2 text in full.

Removed.

@ch3yne ch3yne changed the title [#1113] improvement(ui): TS support and improve global request handing [#1113] improvement(UI): TS support and improve global request handing Jan 3, 2024
@qqqttt123
Copy link
Contributor

Does this pull request need to back port branch 0.3?

@ch3yne ch3yne changed the title [#1113] improvement(UI): TS support and improve global request handing [#1113][#1224] improvement(UI): TS support and improve global request handing Jan 4, 2024
@ch3yne
Copy link
Contributor Author

ch3yne commented Jan 4, 2024

Does this pull request need to back port branch 0.3?

I think it's needed.

@jerryshao jerryshao added branch-0.3 need backport Issues that need to backport to another branch labels Jan 4, 2024
errMessage = 'The http version does not support the request!'
break

default:
Copy link
Contributor

Choose a reason for hiding this comment

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

Are these error messages all defined messages? I think we (Gravitino) have our error message, should we use the Gravitino's returned error message?

jerryshao
jerryshao previously approved these changes Jan 4, 2024
Copy link
Contributor

@jerryshao jerryshao left a comment

Choose a reason for hiding this comment

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

LGTM.

@jerryshao
Copy link
Contributor

Seems there're some building issues we need to fix. @ch3yne

@jerryshao
Copy link
Contributor

@justinmclean can you please help to check Iicense related things, thanks!

@jerryshao jerryshao merged commit 5588080 into apache:main Jan 4, 2024
7 checks passed
github-actions bot pushed a commit that referenced this pull request Jan 4, 2024
… handing (#1308)

### What changes were proposed in this pull request?
 
1. Support TypeScript
2. Improve global reauest handing and add error messages

### Why are the changes needed?

Fix: #1113 
Fix: #1224 
Fix: #1225 

### Does this PR introduce _any_ user-facing change?

N/A

### How was this patch tested?

N/A
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
need backport Issues that need to backport to another branch
Projects
None yet
4 participants