dididy commented on PR #5053: URL: https://github.com/apache/zeppelin/pull/5053#issuecomment-3234041360
Thank you for the detailed review. First, I removed build file(**constructorParamsOrderRule.js**) and updated `.gitignore` so that any build files under `tslint-rules/*.js` are not included. I also added a script in package.json to build the files inside tslint-rules, and ensured that this script is executed during postinstall. In other words, when a user runs `postinstall`, `constructorParamsOrderRule.js` file will be generated, and the custom TSLint rule will run based on that file. command are as follows: ```sh npm run build:tslint-rules ``` Regarding the point you raised about optional parameters or the `@Optional` decorator, I updated the rule to handle those cases as well. I confirmed that **json-visualization.ts** is now fixed correctly. If other edge cases come up, I’ll make sure to address them. As for non-Angular classes—are there any being used in this project? From what I’ve seen in **zeppelin-web-angular**, constructors are only used within Angular classes. Since Zeppelin is a large open-source project, I believe having a predictable and consistent order is more important than individual convenience. That’s also how I interpreted the original proposer(@ParkGyeongTae)’s intention behind suggesting this rule. I’d be curious to hear your thoughts on this point. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: reviews-unsubscr...@zeppelin.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
