fix(publisher): use constant TiUP delivery key and fix YAML import

[wuhuizuo]

Introduce tiupServiceDeliveryCfgKey = "delivery_config_file" and use it to access the TiUP delivery config value. Replace incorrect yaml import from github.com/stretchr/testify/assert/yaml with gopkg.in/yaml.v3.

[gemini-code-assist]

Summary of Changes

Hello @wuhuizuo, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request enhances code maintainability and correctness by centralizing the TiUP delivery configuration key into a constant, preventing potential errors from magic strings. Concurrently, it rectifies an incorrect YAML import, switching to a dedicated and robust YAML parsing library for configuration loading, which improves the reliability of the application's configuration handling.

Highlights

• Constant for TiUP Delivery Key: Introduced a new constant, tiupServiceDeliveryCfgKey, to represent the 'delivery_config_file' key used for accessing TiUP delivery configurations, replacing a hardcoded string.
• YAML Import Correction: Corrected the YAML import path in publisher/pkg/config/load.go from github.com/stretchr/testify/assert/yaml to the proper gopkg.in/yaml.v3 library, ensuring correct and robust YAML parsing.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[ti-chi-bot]

I have already done a preliminary review for you, and I hope to help you do a better job.

Summary
This PR fixes two issues: it introduces a constant tiupServiceDeliveryCfgKey to unify access to the TiUP delivery config key, improving maintainability and reducing hardcoded strings; and it corrects an erroneous YAML import by replacing the invalid "github.com/stretchr/testify/assert/yaml" with the proper "gopkg.in/yaml.v3". The changes are small but crucial for correctness and clarity. The code is clean and straightforward, but there are some minor improvements and best practices to consider.

---

Code Improvements

• Constant Naming Consistency

  • File: publisher/internal/service/impl/tiup/types.go (line 7)
  • Issue: The constant tiupServiceDeliveryCfgKey is named with mixed casing for a constant key string. According to Go convention, constants representing fixed strings are often named in CamelCase or ALL_CAPS for clarity. Also, it might help to clarify that this is a config key string.
  • Suggestion: Rename to TiupServiceDeliveryConfigKey or TiupDeliveryConfigKey for consistency with Go exported constants style if intended for use outside the package, or tiupDeliveryConfigKey if unexported but more readable.

  const tiupDeliveryConfigKey = "delivery_config_file"

• Type Safety and Existence Check

  • File: publisher/internal/service/impl/tiup/service.go (line 31)
  • Issue: Accessing v[tiupServiceDeliveryCfgKey] directly without checking if the key exists could cause a nil interface value, which might lead to unexpected behavior downstream.
  • Suggestion: Use a comma-ok idiom to confirm presence before type assertion:

  if deliveryConfigFile, ok := v[tiupServiceDeliveryCfgKey]; ok {
      if file, ok := deliveryConfigFile.(string); ok {
          // proceed with file
      }
  }

  This prevents nil panics and improves robustness.

---

Best Practices

• Import Cleanup and Grouping

  • File: publisher/pkg/config/load.go (lines 4-9)
  • Issue: The import statement has a leftover from a test assertion package import which was incorrect. The fix replaced it correctly, but the import block could be grouped better for clarity following Go conventions (standard library imports first, then third-party).
  • Suggestion: Organize imports as:

  import (
      "fmt"
      "os"
  
      "gopkg.in/yaml.v3"
  )

  This helps maintain readability and consistency.

• Add Comments for the New Constant

  • File: publisher/internal/service/impl/tiup/types.go (above line 7)
  • Issue: The new constant tiupServiceDeliveryCfgKey lacks a comment explaining its purpose.
  • Suggestion: Add a GoDoc-style comment to clarify its role for future maintainers:

  // tiupServiceDeliveryCfgKey is the configuration key used to access the TiUP delivery config file path.
  const tiupServiceDeliveryCfgKey = "delivery_config_file"

• Testing Coverage for the Config Key Usage

  • Files changed do not show any test updates.
  • Issue: It’s unclear whether the usage of the new constant and the corrected YAML import are covered by existing tests. If not, this might risk regressions.
  • Suggestion: Add or update unit tests in the TiUP service package to verify that the delivery config file is correctly loaded using the new constant, and that YAML unmarshalling works as expected.

---

Critical Issues

No critical bugs, security vulnerabilities, or performance regressions were identified in these specific changes.

---

Summary of Actions:

• Use comma-ok idiom to safely access the map key in service.go.
• Consider renaming the constant for clarity and consistency.
• Add a comment describing the new constant.
• Clean up import grouping in load.go.
• Verify or add tests covering these fixes.

[gemini-code-assist]

Code Review

This pull request introduces two important fixes and a refactoring. It corrects the key used to retrieve the TiUP delivery configuration file path, which was previously using an incorrect camelCase name, and introduces a constant for this key, improving maintainability. Additionally, it replaces a non-functional YAML library import with the standard gopkg.in/yaml.v3, which is a critical fix for configuration loading. The changes are correct and improve the overall quality of the code.

[wuhuizuo]

/approve

[ti-chi-bot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: wuhuizuo

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

The pull request process is described here

Details

Needs approval from an approver in each of these files:

• OWNERS [wuhuizuo]

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