Skip to content

Conversation

@Pinont
Copy link
Owner

@Pinont Pinont commented Nov 7, 2025

  • Fix version 1.3.2 cannot get main path on api development
  • Fix Api version null when compile api into plugin
  • Update into library only and plugin library
  • Update Release.yml Action

@Pinont Pinont requested a review from Copilot November 7, 2025 07:34
@Pinont Pinont merged commit 1f9cc43 into main Nov 7, 2025
4 checks passed
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR refactors API version management to use a configuration file approach instead of hardcoded values, updates the Maven build to support optional shading, and streamlines the release workflow.

  • Moved API version from hardcoded string in CorePlugin.getAPIVersion() to a configurable YAML file with Maven resource filtering
  • Converted Maven shade plugin to an optional profile (with-shade) that must be explicitly activated
  • Updated GitHub Actions release workflow to use the shade profile and simplify artifact handling

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
src/main/resources/api-version.yml New configuration file containing API version with Maven placeholder for build-time substitution
src/main/java/com/github/pinont/singularitylib/plugin/CorePlugin.java Removed hardcoded getAPIVersion() method and updated to use Common.getAPIVersion()
src/main/java/com/github/pinont/singularitylib/api/utils/Common.java Added new getAPIVersion() method that reads version from configuration file
pom.xml Moved shade plugin from default build to optional with-shade profile and updated version to 1.3.3
.github/workflows/release.yml Updated build command to activate shade profile, simplified release artifact handling, and fixed indentation

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +120 to +121
echo "Tag ${TAG} already exists unexpectedly (race condition?)."
exit 1
Copy link

Copilot AI Nov 7, 2025

Choose a reason for hiding this comment

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

Indentation is inconsistent. The closing fi statement should be aligned with the opening if statement. This should be:

if git rev-parse -q --verify "refs/tags/${TAG}" >/dev/null; then
  echo "Tag ${TAG} already exists unexpectedly (race condition?)."
  exit 1
fi
Suggested change
echo "Tag ${TAG} already exists unexpectedly (race condition?)."
exit 1
echo "Tag ${TAG} already exists unexpectedly (race condition?)."
exit 1

Copilot uses AI. Check for mistakes.
Comment on lines +38 to +42
public static String getAPIVersion() {
ConfigManager apiConfig = new ConfigManager(CorePlugin.getInstance(), "api-version.yml");
String version = apiConfig.getConfig().getString("api-version", "1.0.0");
return "V-" + version;
}
Copy link

Copilot AI Nov 7, 2025

Choose a reason for hiding this comment

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

The ConfigManager creates an empty file when the configuration doesn't exist (see lines 32-36 in ConfigManager.java), but it doesn't copy the default content from src/main/resources/api-version.yml. This means when getAPIVersion() is first called, it will create an empty api-version.yml file in the plugin's data folder, and the version will default to "1.0.0" instead of the actual version specified in the resource file.

To properly load the resource file with Maven-filtered version, you should either:

  1. Use plugin.saveResource("api-version.yml", false) before creating the ConfigManager to copy the resource file
  2. Or read the version directly from the plugin's JAR resources using plugin.getResource("api-version.yml")

Example fix:

public static String getAPIVersion() {
    try (InputStream is = CorePlugin.getInstance().getResource("api-version.yml")) {
        if (is != null) {
            YamlConfiguration config = YamlConfiguration.loadConfiguration(new InputStreamReader(is));
            String version = config.getString("api-version", "1.0.0");
            return "V-" + version;
        }
    } catch (IOException e) {
        // Log error
    }
    return "V-1.0.0";
}

Copilot uses AI. Check for mistakes.
target/*-sources.jar
target/*-javadoc.jar
main-artifact.jar
`target/*.jar`
Copy link

Copilot AI Nov 7, 2025

Choose a reason for hiding this comment

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

The file path pattern should use glob pattern syntax, not backticks. In YAML, backticks don't have special meaning for pattern matching. This should be:

files: |
  target/*.jar

Without the backticks, the glob pattern will work correctly to match all JAR files in the target directory.

Suggested change
`target/*.jar`
target/*.jar

Copilot uses AI. Check for mistakes.
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.

2 participants