remove config.yaml and replace it with github variable - #2
Open
goku-kamehameha wants to merge 4 commits into
Open
remove config.yaml and replace it with github variable#2goku-kamehameha wants to merge 4 commits into
goku-kamehameha wants to merge 4 commits into
Conversation
…ent variable XRAY_SUBREFINER_CONFIG and updating README for clarity
…andling, and update README for clarity
…ests for multiline YAML support
There was a problem hiding this comment.
Pull request overview
This PR removes the repository-shipped config.yaml and updates the CLI + GitHub Actions workflow to load configuration from a GitHub Actions variable (XRAY_SUBREFINER_CONFIG) or an explicitly provided -config path, aligning CI runs with repo/environment configuration management.
Changes:
- Removed
config.yamlfrom the repository and updated documentation to reflect externalized configuration. - Updated
cmd/xraysubrefinerto resolve config from-configand/orXRAY_SUBREFINER_CONFIG, with parsing support for inline YAML. - Updated the normalize workflow to pass
XRAY_SUBREFINER_CONFIGviavarsand removed the-config config.yamlinvocation; added unit tests for config loading behavior.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| README.md | Documents the move to XRAY_SUBREFINER_CONFIG and the lack of a default config.yaml. |
| config.yaml | Removes the previously committed default configuration file. |
| cmd/xraysubrefiner/main.go | Adds env/flag-based config resolution and supports inline YAML config sources. |
| cmd/xraysubrefiner/main_test.go | Adds tests covering config source resolution and parsing/file loading. |
| .github/workflows/normalize.yml | Runs without config.yaml, injects XRAY_SUBREFINER_CONFIG from GitHub variables, and updates push remote URL. |
Suppressed comments (2)
cmd/xraysubrefiner/main.go:147
resolveConfigSourcecurrently falls back to the env var when-configwas explicitly provided but left empty (e.g.-config ""). That makes an explicit flag value not reliably take precedence and can silently load the wrong config. Consider treating an explicit-configas authoritative (even if empty), lettingloadConfigreturn a clear error for the empty case.
func resolveConfigSource(explicitConfig bool, flagValue, envValue string) string {
flagValue = strings.TrimSpace(flagValue)
if explicitConfig && flagValue != "" {
return flagValue
}
cmd/xraysubrefiner/main.go:198
looksLikeInlineYAMLtreats any string containing:as inline YAML. That is very broad and will flag many path-like values (e.g. Windows drive paths or other colon-containing filenames) as YAML. Tightening the heuristic reduces false positives while still supporting common single-line YAML likekey: value.
func looksLikeInlineYAML(source string) bool {
if strings.Contains(source, "\n") || strings.Contains(source, "\r") {
return true
}
if strings.Contains(source, ":") || strings.Contains(source, "{") || strings.Contains(source, "[") {
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+154
to
+178
| func loadConfig(source string) (*Config, error) { | ||
| source = strings.TrimSpace(source) | ||
| if source == "" { | ||
| return nil, fmt.Errorf("config source is empty") | ||
| } | ||
|
|
||
| if looksLikeInlineYAML(source) { | ||
| return parseConfig([]byte(source)) | ||
| } | ||
|
|
||
| if info, err := os.Stat(source); err == nil { | ||
| if info.IsDir() { | ||
| return nil, fmt.Errorf("config path %q is a directory", source) | ||
| } | ||
| b, err := os.ReadFile(source) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| return parseConfig(b) | ||
| } else if err != nil && !os.IsNotExist(err) { | ||
| return nil, err | ||
| } | ||
|
|
||
| return nil, fmt.Errorf("config source %q not found", source) | ||
| } |
Comment on lines
+68
to
72
| configSource := resolveConfigSource(explicitConfig, *cfgPath, os.Getenv(configEnvVar)) | ||
| cfg, err := loadConfig(configSource) | ||
| must(err) | ||
|
|
||
| client := &http.Client{Timeout: *timeout} |
| @@ -80,6 +80,11 @@ A ready-to-use workflow is included at `.github/workflows/normalize.yml`: | |||
| - Triggers every hour (`cron: "0 * * * *"`). | |||
This file contains hidden or 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
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.
Create the github variable first