-
Notifications
You must be signed in to change notification settings - Fork 5.4k
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
Generate JS files with Maven build #22645
Conversation
Codenotify: Notifying subscribers in CODENOTIFY files for diff 0edb84c...1d43c61.
|
cc @elharo those compiled JS files are removed! |
7d9c892
to
0890673
Compare
Just updated the node version to the latest LTS of v18. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the update of CONTRIBUTING.md! A few nits and very minor phrasing suggestions.
Compile the JS files with the Maven build and no need to check-in and verify the JS files in a PR. Also update the documents accordingly. Signed-off-by: Yihong Wang <yh.wang@ibm.com>
0890673
to
1d43c61
Compare
@steveburnett again thanks for the comments and please review my updates that address your comments. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Really nice work! This is an awesome code cleanup. Thank you @yhwang!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! (docs)
Pull updated branch, new build of docs. Everything looks good. Thanks!
Description
Compile the JS files with the Maven build and no
need to check in and verify the JS files in a PR.
Also, update the documents accordingly and remove
all the compiled JS files.
related: #21062 item 6
Motivation and Context
Checking in the compiled JS files causes confusion and no way to review the change. Integrating
yarn
intothe Maven build solves the issue.
Impact
No need to check in the compiled JS files in a PR that changes any of the Presto Console code.
Test Plan
The GH action shall pass without any issues. I also manually built the code and verified the Presto Console worked properly.
Contributor checklist
Release Notes
Please follow release notes guidelines and fill in the release notes below.