fix: legacy pkg publish #533
Merged
Conversation
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
Member
elrrrrrrr
commented
Jun 27, 2023
•
elrrrrrrr
commented
- 🧶 Update the validation rules, allow to publish existing packages.
- 🧶 更新校验规则，允许存量包修改
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| if (!pkg) { | ||
| const errors = (validateResult.errors || validateResult.warnings).join(', '); | ||
| throw new UnprocessableEntityError(`package.name invalid, errors: ${errors}`); | ||
| } | ||
| } | ||
| const versions = Object.values(pkg.versions); | ||
| if (versions.length === 0) { |
There was a problem hiding this comment.
这段代码的功能是发布一个npm包，以下是我的代码审核：
- 第7行：该行是多余的，应该将其删除。
- 第10-14行：使用validate-npm-package-name来验证包名是否有效，在没有错误或警告时抛出一个UnprocessableEntityError。
- 第15-21行：如果包已经存在，则仍然允许发布。在这种情况下，它会查找包的信息并打印出来，如果找不到该包，则抛出UnprocessableEntityError。
- 第22-25行：获取包中所有版本的值，并检查它们是否为0。
以下是一些改进建议：
- 需要添加一些注释以解释某些特定代码段的目的和功能。这将使代码更易于理解和维护。
- 第19行中的“pkg”重新赋值，覆盖了类级别变量，这可能会导致不可预测的结果。应该给这个变量一个不同的名称，以避免混淆。
- 在findPackage()方法上引入try-catch块作为保护，以处理数据库操作中的任何异常或错误情况。
- 应该使用ESLint等工具检查代码风格和代码规范，并且执行单元测试和端到端测试以确保代码正确性和性能满足要求。
elrrrrrrr
force-pushed
the
legacy-pkg-publish
branch
from
219831e
to
f3e2bff
Compare
| if (!pkg) { | ||
| const errors = (validateResult.errors || validateResult.warnings).join(', '); | ||
| throw new UnprocessableEntityError(`package.name invalid, errors: ${errors}`); | ||
| } | ||
| } | ||
| const versions = Object.values(pkg.versions); | ||
| if (versions.length === 0) { |
There was a problem hiding this comment.
这段代码的主要问题在于变量名 pkg 的重复使用。在函数开始时，fullname 不等于 pkg.name 时会抛出异常，而后面检查 pkg.name 的有效性时，又重新定义了同名的 pkg 变量，在两个地方都使用了它。这会导致第二次 pkg 的定义覆盖了第一次调用时传递进来的参数 pkg，可能会引入潜在的错误。
此外，当 validateResult.validForNewPackages 不为真时，记录了一个日志信息，并不会终止程序运行。如果自动化发布新包时发现了无效包名称，建议不要继续执行，并考虑添加更详细的错误提示信息。
改善建议：
- 避免重复使用同一个变量名，在第二个
pkg变量定义前通过重命名或其他方式解决。 - 当检测到无效包名称时抛出异常并中断程序的执行。必要时给出更具体的错误信息以帮助诊断问题。
| assert(res.status === 201); | ||
|
|
||
| }); | ||
|
|
||
| it('should 422 when attachment data format invalid', async () => { | ||
| let pkg = await TestUtil.getFullPackage({ | ||
| attachment: { |
There was a problem hiding this comment.
这段代码进行了一个单元测试的编写，测试了一个保存依赖包版本的控制器。具体的变化如下：
- 增加了一个从
app/core/service/PackageManagerService模块中获取PackageManagerService对象的引入。 - 增加了一个测试方法，用于测试允许发布已存在的npm包。
- 原本最后一个测试方法表示检查附件数据格式是否正确，被截断了。
由于只有部分代码片段，所以我无法对所有风险和优化建议进行全面评估。但是，根据目前提供的信息，似乎没有明显的错误或潜在风险。但是，我们可以考虑以下改进：
-
在导入
dayjs之前将所有其他导入放在一起。 -
考虑拆分较长的代码行，以使每个行都容易阅读。
-
可以考虑更多（甚至完整）的测试示例，以覆盖更多可能的情况并帮助识别潜在问题。
-
考虑为旧的或过时的模块添加警告标志，并考虑更新它们。
-
报告奇怪或异常情况，并捕获可能会发生的异常，避免程序崩溃。
Codecov Report
@@ Coverage Diff @@
## master #533 +/- ##
==========================================
- Coverage 97.03% 96.98% -0.06%
==========================================
Files 174 174
Lines 16554 16561 +7
Branches 2169 2167 -2
==========================================
- Hits 16063 16061 -2
- Misses 491 500 +9
|
fengmk2
approved these changes
Jun 27, 2023
fengmk2
pushed a commit
that referenced
this pull request
Jun 27, 2023
[skip ci] ## [3.34.8](v3.34.7...v3.34.8) (2023-06-27) ### Bug Fixes * legacy pkg publish ([#533](#533)) ([20ffba8](20ffba8))
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.