-
Notifications
You must be signed in to change notification settings - Fork 87
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
fix: update source registry #537
Merged
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -179,6 +179,10 @@ describe('test/port/controller/package/SavePackageVersionController.test.ts', () | |
pkgEntity.registryId = ''; | ||
await packageRepository.savePackage(pkgEntity!); | ||
|
||
res = await app.httpRequest() | ||
.get(`/${pkg.name}`); | ||
assert.equal(res.body._source_registry_name, 'default'); | ||
|
||
pkg = await TestUtil.getFullPackage({ name, version: '2.0.0' }); | ||
res = await app.httpRequest() | ||
.put(`/${pkg.name}`) | ||
|
@@ -189,6 +193,10 @@ describe('test/port/controller/package/SavePackageVersionController.test.ts', () | |
|
||
pkgEntity = await packageRepository.findPackage('@cnpm', 'publish-package-test'); | ||
assert(pkgEntity?.registryId); | ||
|
||
res = await app.httpRequest() | ||
.get(`/${pkg.name}`); | ||
assert.equal(res.body._source_registry_name, 'self'); | ||
}); | ||
|
||
it('should publish on user custom scopes', async () => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 对于给定的代码补丁,以下是一个简要的代码审查:
改进建议:
风险:
请注意,这只是一个简要的代码审查意见,具体的问题和改进取决于代码的实际运行环境和需求。 |
||
|
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
这段代码主要是在
PackageManagerService
类中的一个方法中进行了修改。我将逐步解释每个修改并提出改进建议:在第一处修改中,原先的代码是判断
cmd.packageJson._source_registry_name
是否存在,如果不存在则获取registry
并将其赋值给_source_registry_name
。现在的修改将获取registry
的部分移到了条件判断之外,并且不再进行存在性的检查,而是直接将获取到的registry.name
赋值给cmd.packageJson._source_registry_name
。改进建议:这样的修改使得代码更加简洁和一致。条件判断是否存在
_source_registry_name
可以被移除,这样无论_source_registry_name
是否存在都会进行赋值操作。在调用this.getSourceRegistry(pkg)
之前,可以添加对pkg
是否存在的检查来避免潜在的错误。第二处修改与第一处类似,在满足条件的情况下,将
registry?.name
赋值给data._source_registry_name
。改进建议:与第一处修改相同,可以移除对
_source_registry_name
存在性的检查,直接进行赋值操作。总体而言,这些修改使代码更简洁、易读,并且保持了一致性。没有明显的错误风险,但仍然可以根据实际需求进一步优化和简化代码。