generator
generator copied to clipboard
refactor: move out the `loadTemplateConfig` from `generator.js`
Description
- Moved the loadTemplateConfig function to separate file for clean code.
- Renamed and moved the validation function inside template config
Related issue(s) Resolves #1582
Summary by CodeRabbit
-
Refactor
- Streamlined template configuration loading and default parameter handling in generators for improved maintainability without changing user experience.
-
Chores
- Updated internal import paths for consistency and clarity; no impact on functionality or user experience.
⚠️ No Changeset found
Latest commit: 6e7077ff5d63e16e0fb58c05a7794588faea51b0
Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.
This PR includes no changesets
When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types
Click here to learn what changesets are, and how to add one.
Click here if you're a maintainer who wants to add a changeset to this PR
"""
Walkthrough
The changes refactor the template configuration loading logic in the generator by removing internal methods from the Generator class and introducing new external asynchronous loader functions in a dedicated module. Associated import paths and test imports are updated to reflect this modularization, with no other logic or control flow changes.
Changes
| Files/Paths | Change Summary |
|---|---|
| apps/generator/lib/generator.js | Removed internal private async methods loadTemplateConfig and loadDefaultValues; replaced with wrappers calling external implementations; updated imports accordingly. |
| apps/generator/lib/templateConfig/loader.js | New module exporting async functions loadTemplateConfig and loadDefaultValues to load and process template configuration and default parameter values. |
| apps/generator/lib/templateConfig/validator.js | Updated import paths for utility modules to use parent directory references. |
| apps/generator/test/generator.test.js | Updated import paths for template config validator module in mocks and test setup. |
| apps/generator/test/templateConfigValidator.test.js | Updated import path for validateTemplateConfig function. |
| apps/generator/lib/mocks/templateConfigValidator.js | Updated mock import path for template config validator. |
Assessment against linked issues
| Objective | Addressed | Explanation |
|---|---|---|
Move loadTemplateConfig out of generator.js into a new templateConfig/loader.js module (#1582) |
✅ | |
Structure template config logic into templateConfig folder with validator.js and loader.js (#1582) |
✅ |
Assessment against linked issues: Out-of-scope changes
No out-of-scope changes identified.
Possibly related PRs
- asyncapi/generator#1485: Introduced the original support for
.ageneratorrcand updated the template config loading logic, which is now being modularized in this PR. """
📜 Recent review details
Configuration used: .coderabbit.yaml Review profile: CHILL Plan: Pro
📥 Commits
Reviewing files that changed from the base of the PR and between fe4253761ab805de09c7edaeeb7d74da6ec1ea9a and cc074811e9d03d583c34aef8c8842028ee4ec2da.
📒 Files selected for processing (2)
apps/generator/lib/generator.js(2 hunks)apps/generator/lib/templateConfig/loader.js(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/generator/lib/generator.js
🔇 Additional comments (6)
apps/generator/lib/templateConfig/loader.js (6)
1-7: LGTM! Clean imports and appropriate constants.The imports are well-organized and the
CONFIG_FILENAMEconstant improves maintainability.
14-26: Excellent input validation implementation.The function properly validates the input parameter with comprehensive checks for existence, type, and directory status. This addresses potential runtime errors early.
29-41: Well-structured fallback mechanism for configuration loading.The function correctly attempts to load from
.ageneratorrcfirst and provides appropriate error logging when it fails, allowing graceful fallback topackage.json.
43-56: Robust package.json fallback with proper error handling.The fallback logic correctly attempts to load configuration from
package.jsonand handles both file reading and JSON parsing errors gracefully. The function appropriately returns an empty object when both configuration sources fail.
63-77: Efficient default value implementation using property getters.The function elegantly uses
Object.definePropertywith getters for lazy evaluation of default values, ensuring defaults are only applied to undefined parameters and computed on access rather than upfront.
79-82: Clean module exports following Node.js conventions.The exports are clear and follow standard module patterns, making both functions available for external use.
✨ Finishing Touches
- [ ] 📝 Generate Docstrings
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
🪧 Tips
Chat
There are 3 ways to chat with CodeRabbit:
‼️ IMPORTANT Auto-reply has been disabled for this repository in the CodeRabbit settings. The CodeRabbit bot will not respond to your replies unless it is explicitly tagged.
- Files and specific lines of code (under the "Files changed" tab): Tag
@coderabbitaiin a new review comment at the desired location with your query. Examples:@coderabbitai explain this code block.@coderabbitai modularize this function.
- PR comments: Tag
@coderabbitaiin a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:@coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.@coderabbitai read src/utils.ts and explain its main purpose.@coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.@coderabbitai help me debug CodeRabbit configuration file.
Support
Need help? Create a ticket on our support page for assistance with any issues or questions.
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.
CodeRabbit Commands (Invoked using PR comments)
@coderabbitai pauseto pause the reviews on a PR.@coderabbitai resumeto resume the paused reviews.@coderabbitai reviewto trigger an incremental review. This is useful when automatic reviews are disabled for the repository.@coderabbitai full reviewto do a full review from scratch and review all the files again.@coderabbitai summaryto regenerate the summary of the PR.@coderabbitai generate docstringsto generate docstrings for this PR.@coderabbitai generate sequence diagramto generate a sequence diagram of the changes in this PR.@coderabbitai resolveresolve all the CodeRabbit review comments.@coderabbitai configurationto show the current CodeRabbit configuration for the repository.@coderabbitai helpto get help.
Other keywords and placeholders
- Add
@coderabbitai ignoreanywhere in the PR description to prevent this PR from being reviewed. - Add
@coderabbitai summaryto generate the high-level summary at a specific location in the PR description. - Add
@coderabbitaianywhere in the PR title to generate the title automatically.
Documentation and Community
- Visit our Documentation for detailed information on how to use CodeRabbit.
- Join our Discord Community to get help, request features, and share feedback.
- Follow us on X/Twitter for updates and announcements.
Hey @ItsRoy69 Have you run the test by npm run test command?
@derberg can you please start the workflow?
@ItshMoh Yes, while I ran npm run test, @derberg I found issues related to other files like SendOperation.js, MessageExamples.js and AvailableOperations.js for which many cases are failing (not related to loadTemplateConfig functions), should I create a separate issue/pull request fixing these ?
@ItsRoy69 go through these issues. Change the path of the `templateConfigValidator' in generator.test.js and other files where it is occuring. The path has been changed so change its path where it has occured.
here are some of the issues.
@ItsRoy69
@ItshMoh hey, updated the path accordingly and tested out , its passing now.
@ItsRoy69 There are also some issues in dart client. fix it.
These are not issues with the generator. It may be because of new changes.
The snapshots are obselete, generate new ones.
@ItsRoy69 also run npm run lint after the changes.
if there is any error npm run lint:fix.
And please also go through the https://github.com/asyncapi/generator/blob/master/CONTRIBUTING.md
npm run test
sure, will go through it , noticed many testings are failing in other files, looking into it
@ItshMoh the integration issue is separate issue, I guess that can be handled in separate PR as its quite many changes required for that which is not related to template generator error
@ItshMoh the integration issue is separate issue, I guess that can be handled in separate PR as its quite many changes required for that which is not related to template generator error
@ItsRoy69 the integration issue is not the separate issue. it is due to the changes we made in this PR. So we need to fix it here. Otherwise how is it gonna be merged?. Look for it
@ItsRoy69 modify the generator.test.js file. Where the tests are failing due to
gen.loadTemplateConfig and gen.loadDefaultValues these are now not in the generator class.
Fixed the issues for which testing were failing after running npm run test, npm run lint:fix, npm run lint
@ItsRoy69 remove the conflicts also.
@derberg can you please look here mainly the decoulpling of the loadTemplateConfig from the generator.js
@derberg can you run the workflow when you are free, and also if you can review the decoupling part.
Hey @ItsRoy69 Can you please refactor the code as @derberg suggested?
Yes, working on it
@ItshMoh refactored the code accordingly, tested npm run lint, npm run test and npm run lint:fix as well
hey @Adi-204 Can you please review this PR? It would be a great help.
@ItshMoh I looked at PR and it is looking good, but I think @derberg is best person to guide with the changes done in app/generator.
Quality Gate passed
Issues
3 New issues
0 Accepted issues
Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code
@ItsRoy69 hey, do you plan to continue working on this one?
closing, no response from contributor
@ItsRoy69 if you still want to work on it, feel free to open a new PR whenever ready