-
Notifications
You must be signed in to change notification settings - Fork 1
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
将与配置相关的内容迁移到 scalabot-meta 模块。 #8
Conversation
通过将配置迁移到单独的模块, 可以方便使用其他程序扩展 ScaleBot, 而不仅仅是让 ScaleBot 成为扩展的平台. BREAKING CHANGE: 与配置有关的 Class 移动到了 scalabot-meta 模块. 目前仅所有配置类(以 `Config` 结尾的 Class)和相应的序列化类(以 `Serializer` 结尾的)都迁移到了 meta 模块, 但其工具方法则作为扩展函数保留在 app 模块中. 这么做的好处是为了方便其他应用(例如 ScalaBot 外部管理程序)根据需要生成配置文件. scalabot-meta 将会作为依赖项发布, 可根据需要获取 ScalaBot-meta 生成 ScalaBot 的配置. 此次改动普通用户无需迁移.
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.
已标记部分问题,晚些再修正。
迁移前, createDefaultRepositoryId 方法和 checkRepositoryLayout 方法已经是 Private 了, 迁移中出现差错导致变更为 internal, 现已修复. Pull Request #8
AbstractArtifact 已经有官方的 toString 实现了, 故不再多此一举. 同时, 如果有不基于 AbstractArtifact 的 Artifact 实现, 将会转换成 DefaultArtifact 并直接使用 toString.
增加对 Serializer 中可能抛出的异常(例如 MalformedURLException, IllegalArgumentException)包装成 JsonParseException, 以避免异常类型混乱的问题.
为了保证扩展中命令的权限判断有效性, 故移除 BotAccount 中 creatorId 字段的默认值, 此改动将要求用户提供准确的 Bot 创建者 Id. 这个改动拖得越久, 影响的范围就越大. 另外, 为 BotConfig 中的 extensions 属性和 proxy 属性增加默认值, 以减少意义重复的情况(例如当用户没设置 proxy 属性时提供一个 type 为 NO_PROXY 的 ProxyConfig, 无需判断是否为 null).
通过调整部分属性的 null-safety 特性, 移除了部分 non-null 判断, 略微(真的很略微)提高了性能(虽然仅限于启动).
更改 enabled 的默认值, 以防止意外启动 Bot. 同时让 bot.json 在初始化时设为 true, 方便用户改完就能启动.
由于 Gson 的解析方式不能正确处理 Kotlin 的 null-safety 属性, 因此添加两个 Serializer, 手动解析 Json 以避开这个问题. Close #9
将 BotAccountTest 合并入 ConfigsTest, 方便归类测试, 并补充其他配置类的 JSON 解析; 此部分独立于 Serializer 以防止后续更改出现潜在的解析错误.
为了归类单元测试, 所以将 ArtifactSerializerTest 合并到 SerializersKtTest; 添加 ProxyConfig 和 BotConfig 的单元测试类.
UsernameAuthenticatorTest 所测试的 UsernameAuthenticator 是属于 config 包的, 所以修正了一下这个问题.
️✅ There are no secrets present in this pull request anymore.If these secrets were true positive and are still valid, we highly recommend you to revoke them. 🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request. |
由于默认值未及时变更, 导致出现默认值与预期不符的情况; 目前已调整了新的默认值获取方式, 以便于后续调整默认值.
已确认机密信息泄露警告为误报。 |
为 BotConfig 的完整序列化测试添加 Artifact 值, 覆盖解析 BotConfig 所涉及的 Artifact 序列化.
为 ProxyType 增加 HTTPS 类型, 同时为 Aether 增加 Https 代理支持, 方便用户使用现有的公开代理下载依赖包.
…Test. 将测试类移动到对应的文件中.
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.
初步检查没有什么问题。
Test 注解将统一使用 kotlin.test.Test, 这么做可以保持兼容性; 将 MavenRepositoryConfigSerializerTest.`json primitive deserialize test` 中的两段代码顺序调整一下, 以避免出现歧义.
由于在 MavenRepositoryConfigSerializer 反序列化中过滤了 Json 的类型, 导致用户在配置中使用了错误的 Json 数据类型将不会有任何错误信息. 该改动已解决该问题.
补充最少参数的反序列化测试项, 以确保在 Json 属性缺失的情况下依然能正确反序列化出正确的对象.
补充新的断言状况, 以保证功能正常使用.
由于 `MavenRepositoryConfig.authentication` 有关联的序列化器, 因此不可忽略对该属性进行检查; 现已补充单元测试用例以确保反序列化结果正确.
大概是没有其他问题的了,如果有,那应该跟这个 pr 无关。(累死嘞) |
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.
已检查没有问题,如后续无需增加改动,将可以直接合并。
由于先前的更新忘记补充文档了, 所以补充一下.
为 meta 模块增加 Maven 构件发布配置, 但仍需本地发布(因为要 GPG 签名).
基本上是 OK 的了 |
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.
已确定无问题。
通过将配置迁移到单独的模块,可以方便使用其他程序扩展 ScaleBot,而不仅仅是让 ScaleBot 成为扩展的平台。
BREAKING CHANGE:与配置有关的 Class 移动到了 scalabot-meta 模块。
目前仅所有配置类(以
Config
结尾的 Class)和相应的序列化类(以Serializer
结尾的)都迁移到了 meta 模块,但其工具方法则作为扩展函数保留在 app 模块中。这么做的好处是为了方便其他应用(例如 ScalaBot 外部管理程序)根据需要生成配置文件。
scalabot-meta 将会作为依赖项发布,可根据需要获取 ScalaBot-meta 生成 ScalaBot 的配置。
此次改动普通用户无需迁移。
另外补充一下:如果 meta 模块顺利合并,那么将可以尝试实现 Issue #4 。