-
Notifications
You must be signed in to change notification settings - Fork 55
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
feat: impl egg-bin dal gen #257
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThis update introduces a new feature for generating data access layer (DAL) code, enhancing the Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
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 as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
@coderabbitai review |
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.
Review Status
Actionable comments generated: 2
Configuration used: CodeRabbit UI
Files ignored due to path filters (4)
package.json
is excluded by:!**/*.json
test/fixtures/dal/app/modules/dal/package.json
is excluded by:!**/*.json
test/fixtures/dal/package.json
is excluded by:!**/*.json
test/fixtures/dal/tsconfig.json
is excluded by:!**/*.json
Files selected for processing (8)
- .gitignore (1 hunks)
- README.md (1 hunks)
- lib/cmd/dal.js (1 hunks)
- lib/cmd/dal/gen.js (1 hunks)
- lib/dal-gen.js (1 hunks)
- test/fixtures/dal/app/modules/dal/Bar.ts (1 hunks)
- test/fixtures/dal/app/modules/dal/Foo.ts (1 hunks)
- test/lib/cmd/dal.test.js (1 hunks)
Files skipped from review due to trivial changes (1)
- .gitignore
Additional comments: 5
test/fixtures/dal/app/modules/dal/Bar.ts (1)
- 1-19: The class definition and decorators usage are correctly implemented for ORM mapping. Consider adding documentation comments to the class and its fields for better maintainability.
lib/cmd/dal.js (1)
- 6-17: The
DalCommand
class is correctly implemented. Consider adding an English description or a comment for thedescription
getter to improve accessibility for non-Chinese speaking developers.test/fixtures/dal/app/modules/dal/Foo.ts (1)
- 1-23: The class definition and decorators usage, including the unique index, are correctly implemented. Consider adding documentation comments to the class and its fields for better maintainability.
lib/cmd/dal/gen.js (1)
- 5-39: The
DalGenCommand
class is well-implemented. Consider adding more detailed error handling or logging in therun
method to improve debugging and maintainability.lib/dal-gen.js (1)
- 1-36: The DAL code generation script is correctly implemented. Consider adding comments explaining the choice of exit codes (
0
for success,1
for failure) for clarity and maintainability.
### dal | ||
|
||
Generate code for @eggjs/tegg-dal-plugin | ||
|
||
```bash | ||
egg-bin dal gen | ||
``` | ||
|
||
dal document please read [tegg doc](https://github.com/eggjs/tegg/tree/master/plugin/dal). |
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.
The new section on the dal
command is clear and concise, providing straightforward instructions on how to use the egg-bin dal gen
command and where to find further documentation. However, to maintain consistency with the rest of the document, consider starting the description with an uppercase letter and adding a comma after "dal document please read" for better readability.
- dal document please read [tegg doc](https://github.com/eggjs/tegg/tree/master/plugin/dal).
+ Dal document, please read [tegg doc](https://github.com/eggjs/tegg/tree/master/plugin/dal).
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
### dal | |
Generate code for @eggjs/tegg-dal-plugin | |
```bash | |
egg-bin dal gen | |
``` | |
dal document please read [tegg doc](https://github.com/eggjs/tegg/tree/master/plugin/dal). | |
### dal | |
Generate code for @eggjs/tegg-dal-plugin | |
```bash | |
egg-bin dal gen |
Dal document, please read tegg doc.
</details>
<!-- suggestion_end -->
<!-- This is an auto-generated comment by CodeRabbit -->
describe('test/lib/cmd/dal.test.js', () => { | ||
const eggBin = require.resolve('../../../bin/egg-bin.js'); | ||
const cwd = path.join(__dirname, '../../fixtures/dal'); | ||
|
||
afterEach(mm.restore); | ||
|
||
describe('egg-bin dal gen', () => { | ||
after(async () => { | ||
await fs.rm(path.join(cwd, 'app/modules/dal/dal'), { | ||
recursive: true, | ||
}); | ||
}); | ||
|
||
it('egg-bin dal gen should work', async () => { | ||
await coffee.fork(eggBin, [ 'dal', 'gen' ], { cwd }) | ||
.debug() | ||
.expect('code', 0) | ||
.end(); | ||
|
||
for (const file of [ | ||
'app/modules/dal/dal/dao/BarDAO.ts', | ||
'app/modules/dal/dal/dao/FooDAO.ts', | ||
'app/modules/dal/dal/dao/base/BaseBarDAO.ts', | ||
'app/modules/dal/dal/dao/base/BaseFooDAO.ts', | ||
'app/modules/dal/dal/extension/BarExtension.ts', | ||
'app/modules/dal/dal/extension/FooExtension.ts', | ||
'app/modules/dal/dal/structure/Bar.json', | ||
'app/modules/dal/dal/structure/Bar.sql', | ||
'app/modules/dal/dal/structure/Foo.json', | ||
'app/modules/dal/dal/structure/Foo.sql', | ||
]) { | ||
assert.ok(fs.stat(path.join(cwd, file))); | ||
} | ||
}); | ||
}); | ||
}); |
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.
The test suite for egg-bin dal gen
is well-implemented. However, ensure that the fs.stat
promise is properly awaited or handled to accurately test for the existence of generated files.
- assert.ok(fs.stat(path.join(cwd, file)));
+ assert.ok(await fs.stat(path.join(cwd, file)));
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
describe('test/lib/cmd/dal.test.js', () => { | |
const eggBin = require.resolve('../../../bin/egg-bin.js'); | |
const cwd = path.join(__dirname, '../../fixtures/dal'); | |
afterEach(mm.restore); | |
describe('egg-bin dal gen', () => { | |
after(async () => { | |
await fs.rm(path.join(cwd, 'app/modules/dal/dal'), { | |
recursive: true, | |
}); | |
}); | |
it('egg-bin dal gen should work', async () => { | |
await coffee.fork(eggBin, [ 'dal', 'gen' ], { cwd }) | |
.debug() | |
.expect('code', 0) | |
.end(); | |
for (const file of [ | |
'app/modules/dal/dal/dao/BarDAO.ts', | |
'app/modules/dal/dal/dao/FooDAO.ts', | |
'app/modules/dal/dal/dao/base/BaseBarDAO.ts', | |
'app/modules/dal/dal/dao/base/BaseFooDAO.ts', | |
'app/modules/dal/dal/extension/BarExtension.ts', | |
'app/modules/dal/dal/extension/FooExtension.ts', | |
'app/modules/dal/dal/structure/Bar.json', | |
'app/modules/dal/dal/structure/Bar.sql', | |
'app/modules/dal/dal/structure/Foo.json', | |
'app/modules/dal/dal/structure/Foo.sql', | |
]) { | |
assert.ok(fs.stat(path.join(cwd, file))); | |
} | |
}); | |
}); | |
}); | |
describe('test/lib/cmd/dal.test.js', () => { | |
const eggBin = require.resolve('../../../bin/egg-bin.js'); | |
const cwd = path.join(__dirname, '../../fixtures/dal'); | |
afterEach(mm.restore); | |
describe('egg-bin dal gen', () => { | |
after(async () => { | |
await fs.rm(path.join(cwd, 'app/modules/dal/dal'), { | |
recursive: true, | |
}); | |
}); | |
it('egg-bin dal gen should work', async () => { | |
await coffee.fork(eggBin, [ 'dal', 'gen' ], { cwd }) | |
.debug() | |
.expect('code', 0) | |
.end(); | |
for (const file of [ | |
'app/modules/dal/dal/dao/BarDAO.ts', | |
'app/modules/dal/dal/dao/FooDAO.ts', | |
'app/modules/dal/dal/dao/base/BaseBarDAO.ts', | |
'app/modules/dal/dal/dao/base/BaseFooDAO.ts', | |
'app/modules/dal/dal/extension/BarExtension.ts', | |
'app/modules/dal/dal/extension/FooExtension.ts', | |
'app/modules/dal/dal/structure/Bar.json', | |
'app/modules/dal/dal/structure/Bar.sql', | |
'app/modules/dal/dal/structure/Foo.json', | |
'app/modules/dal/dal/structure/Foo.sql', | |
]) { | |
assert.ok(await fs.stat(path.join(cwd, file))); | |
} | |
}); | |
}); | |
}); |
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.
厉害了。。。
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.
@fengmk2, 谢谢您的认可!如果您有任何问题或需要进一步的帮助,请随时告诉我。 😊
(\__/)
(•ㅅ•) CodeRabbit is happy!
/ づ
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## 5.x #257 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 12 12
Lines 93 93
Branches 16 16
=========================================
Hits 93 93 ☔ View full report in Codecov by Sentry. |
[skip ci] ## [5.16.0](v5.15.1...v5.16.0) (2024-03-22) ### Features * impl egg-bin dal gen ([#257](#257)) ([7195aef](7195aef))
[skip ci] ## [5.16.0](v5.15.1...v5.16.0) (2024-03-22) ### Features * impl egg-bin dal gen ([#257](#257)) ([7195aef](7195aef))
@coderabbitai 提交一个 issue,需要将此功能 pick 到 master 分支 |
Tip For best results, initiate chat on the files or code changes. @fengmk2, I've submitted the issue as requested. It can be found here: Pick DAL Generation Feature to Master Branch. This issue tracks the request to pick the new DAL generation feature introduced in PR #257 to the master branch. Analysis chainScripts executedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Create a GitHub issue in the eggjs/egg-bin repository
gh issue create --repo eggjs/egg-bin --title "Pick DAL Generation Feature to Master Branch" --body "This issue tracks the request to pick the new DAL generation feature introduced in PR https://github.com/eggjs/egg-bin/pull/257 to the master branch. The feature enhances the Egg.js framework's data access layer capabilities by automating the generation of DAL code, making development processes more efficient. Picking this feature to the master branch will ensure its availability for broader use."
Length of output: 549 |
Checklist
npm test
passesAffected core subsystem(s)
Description of change
Summary by CodeRabbit
.gitignore
to exclude.eslintcache
files.