Skip to content
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

refactor: remove LoggingRules interface #422

Closed
wants to merge 1 commit into from
Closed

refactor: remove LoggingRules interface #422

wants to merge 1 commit into from

Conversation

Whale107
Copy link
Contributor

@Whale107 Whale107 commented May 29, 2024

  • 废弃相关接口, 默认启用(可以用 DTK_DISABLED_LOGGING_RULES 禁用)
  • 如果用户通过 DTK_LOGGING_FALLBACK_APPID 指定 了fallbackAppId, 则优先使用 fallbackAppId, 否则使用 dsgAppId

kegechen
kegechen previously approved these changes May 30, 2024
Copy link
Contributor

@kegechen kegechen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

测试一下

@kegechen kegechen requested a review from zccrs May 30, 2024 03:05
Copy link
Member

@zccrs zccrs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这是从 master 分支 cherry-pick 过来的代码?如果需要新接口,使用 DTK 的新版本,不应该往 release/5.6 上加接口。

@deepin-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: Whale107

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

1 similar comment
@deepin-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: Whale107

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@Whale107
Copy link
Contributor Author

这是从 master 分支 cherry-pick 过来的代码?如果需要新接口,使用 DTK 的新版本,不应该往 release/5.6 上加接口。

已更新

- 废弃相关接口, 默认启用(可以用 DTK_DISABLED_LOGGING_RULES 禁用)
- 如果用户通过 DTK_LOGGING_FALLBACK_APPID 指定 了fallbackAppId, 则优先使用 fallbackAppId, 否则使用 dsgAppId
@deepin-ci-robot
Copy link
Contributor

deepin pr auto review

关键摘要:

  • 移除了initLoggingRules函数,但保留了一个新的updateLoggingRules函数,可能需要确认新的逻辑是否与移除的函数功能一致。
  • 使用了qWarning替代了qDebug,这可能会影响日志的输出级别。
  • 删除了initLoggingRules函数中的环境变量逻辑,可能需要确认这是否是有意为之。
  • 删除了registerLoggingRulesWatcher函数中的appId参数,但保留了qWarning消息,这可能会导致混淆。

是否建议立即修改:

  • 是,需要确保新的实现与移除的函数功能一致,并确认移除环境变量逻辑是否符合预期。同时,应该澄清registerLoggingRulesWatcher函数的废弃原因,并更新相关的错误消息。

@Whale107
Copy link
Contributor Author

会从master重新拉release分支,此pr关闭

@Whale107 Whale107 closed this May 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants