Skip to content

fix: remove Install sections from systemd sound services#139

Merged
mhduiy merged 2 commits into
masterfrom
autostart
Jul 3, 2025
Merged

fix: remove Install sections from systemd sound services#139
mhduiy merged 2 commits into
masterfrom
autostart

Conversation

@mhduiy
Copy link
Copy Markdown
Contributor

@mhduiy mhduiy commented Jul 3, 2025

fix: remove Install sections from systemd sound services

  1. Removed [Install] sections from both deepin-login-sound.service and
    deepin-shutdown-sound.service
  2. These services are meant to be triggered manually rather than being
    enabled/started at boot
  3. The WantedBy directives were unnecessary since these are one-shot
    services
  4. Improves system startup performance by not loading unnecessary
    services

fix: 从 systemd 声音服务中移除 Install 部分

  1. 从 deepin-login-sound.service 和 deepin-shutdown-sound.service 中移除
    了 [Install] 部分
  2. 这些服务设计为手动触发而非在启动时自动启用/运行
  3. WantedBy 指令对于一次性服务来说是不必要的
  4. 通过不加载不必要的服务来提高系统启动性能

Summary by Sourcery

Remove installation sections from deepin-login-sound.service and deepin-shutdown-sound.service so they aren’t enabled at boot, reflecting their one-shot, manually triggered nature and improving system startup performance.

Bug Fixes:

  • Remove [Install] sections and WantedBy directives from deepin-login-sound.service and deepin-shutdown-sound.service

Enhancements:

  • Prevent unnecessary loading of these one-shot sound services to improve startup performance

mhduiy added 2 commits July 3, 2025 14:07
1. Removed [Install] sections from both deepin-login-sound.service and
deepin-shutdown-sound.service
2. These services are meant to be triggered manually rather than being
enabled/started at boot
3. The WantedBy directives were unnecessary since these are one-shot
services
4. Improves system startup performance by not loading unnecessary
services

fix: 从 systemd 声音服务中移除 Install 部分

1. 从 deepin-login-sound.service 和 deepin-shutdown-sound.service 中移除
了 [Install] 部分
2. 这些服务设计为手动触发而非在启动时自动启用/运行
3. WantedBy 指令对于一次性服务来说是不必要的
4. 通过不加载不必要的服务来提高系统启动性能
@sourcery-ai
Copy link
Copy Markdown

sourcery-ai Bot commented Jul 3, 2025

Reviewer's Guide

Removes the [Install] sections from the deepin-login-sound and deepin-shutdown-sound systemd unit files and adjusts Debian packaging so they are no longer enabled at boot, ensuring these one-shot services are only invoked manually and reducing startup overhead.

File-Level Changes

Change Details Files
Removed auto-enable directives from sound service units
  • Deleted the entire [Install] section header
  • Removed WantedBy entries
  • Left the units as one-shot triggers only
misc/systemd/system/deepin-login-sound.service
misc/systemd/system/deepin-shutdown-sound.service
Adjusted packaging to stop installing/enabling these unit files by default
  • Removed service install hooks in debian/rules
  • Stopped generating symlinks for these units during packaging
debian/rules

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

Copy link
Copy Markdown

@sourcery-ai sourcery-ai Bot left a comment

Choose a reason for hiding this comment

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

Hey @mhduiy - I've reviewed your changes and they look great!


Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@deepin-ci-robot
Copy link
Copy Markdown

deepin pr auto review

代码审查意见:

  1. 代码简化

    • debian/rules文件中,将多个dh_installsystemd命令简化为一条命令,并使用--no-start选项,这样可以减少重复代码,提高代码的可读性和维护性。
  2. 服务依赖

    • deepin-login-sound.servicedeepin-shutdown-sound.service文件中,移除了[Install]部分,这可能会影响服务的启动顺序和依赖关系。需要确认这些服务是否真的不需要依赖特定的目标(如multi-user.targetgraphical.target),如果需要,应该保留这些依赖关系。
  3. 注释清理

    • debian/rules文件中,移除了不必要的注释,这有助于保持代码的整洁和清晰。
  4. 潜在问题

    • 如果deepin-api-devicedeepin-locale-helperdeepin-sound-theme-player服务确实不需要开机启动,那么移除--no-enable--no-start选项是正确的。但是,如果这些服务在某些情况下需要启动,那么移除这些选项可能会导致问题。
  5. 代码质量

    • 代码简化后,整体代码质量有所提高,但需要确保移除[Install]部分不会对服务的启动和依赖关系产生影响。

综上所述,代码审查意见是积极的,但需要确保移除[Install]部分不会对服务的启动和依赖关系产生影响。如果这些服务确实不需要依赖特定的目标,那么代码简化是合理的。

@deepin-ci-robot
Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: fly602, mhduiy

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

Details 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

@mhduiy mhduiy merged commit 42bf13c into master Jul 3, 2025
28 checks passed
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.

3 participants