Skip to content

Conversation

@ioncakephper
Copy link
Owner

@ioncakephper ioncakephper commented Jun 13, 2025

Description by Korbit AI

What change is being made?

Update configuration file paths in contract-shield.config.json and contract-shield.json, introduce a configuration schema validation in configUtils.js with a new validateConfigSchema method, and re-enable previously commented out tests in transpile.test.js.

Why are these changes being made?

The changes in contract-shield.config.json and contract-shield.json standardize the output directory path format by removing leading './', which ensures consistency across paths. Schema validation is added in configUtils.js to improve configuration reliability and error management, preventing issues with incorrect config structures. Previously commented out tests in transpile.test.js have been re-enabled to ensure the stability and correctness of CLI functionality, guaranteeing robust testing and further validation of the codebase.

Is this description stale? Ask me to generate a new description by commenting /korbit-generate-pr-description

Copy link

@korbit-ai korbit-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.

Review by Korbit AI

Korbit automatically attempts to detect when you fix issues in new commits.
Category Issue Status
Functionality Invalid dirname reference ▹ view
Design Schema Definition Mixed with Validation Logic ▹ view
Files scanned
File Path Reviewed
src/utils/validateConfigSchema.js
src/utils/configUtils.js

Explore our documentation to understand the languages and file types we support and the files we ignore.

Check out our docs on how you can make Korbit work best for you and your team.

Loving Korbit!? Share us on LinkedIn Reddit and X

: null);
const defaultProjectConfig = path.resolve(process.cwd(), CONFIG_FILES.PROJECT);
// const defaultGlobalConfig = path.resolve(os.homedir(), CONFIG_FILES.GLOBAL);
const defaultGlobalConfig = path.resolve(__dirname__, CONFIG_FILES.GLOBAL);
Copy link

Choose a reason for hiding this comment

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

Invalid dirname reference category Functionality

Tell me more
What is the issue?

The code uses 'dirname' (with double underscores) which is an undefined variable. The correct Node.js global variable is '__dirname' (single underscore).

Why this matters

This will cause a ReferenceError when trying to access the undefined variable, preventing the global configuration file from being found.

Suggested change ∙ Feature Preview

Replace 'dirname' with '__dirname':

const defaultGlobalConfig = path.resolve(__dirname, CONFIG_FILES.GLOBAL);
Provide feedback to improve future suggestions

Nice Catch Incorrect Not in Scope Not in coding standard Other

💬 Looking for more details? Reply to this comment to chat with Korbit.

Comment on lines +8 to +13
const requiredFields = ["output"];
for (const field of requiredFields) {
if (!Object.prototype.hasOwnProperty.call(config, field)) {
return false;
}
}
Copy link

Choose a reason for hiding this comment

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

Schema Definition Mixed with Validation Logic category Design

Tell me more
What is the issue?

The validation logic mixes concerns by handling both the required fields list and the validation logic in the same function, making it harder to extend or modify the schema requirements.

Why this matters

Future changes to the configuration schema will require modifying the validation function directly, violating the Open-Closed Principle.

Suggested change ∙ Feature Preview

Separate schema definition from validation logic:

const CONFIG_SCHEMA = {
  required: ['output'],
  types: {
    output: 'string'
  }
};

module.exports = {
  validateConfigSchema: (config) => {
    return validateAgainstSchema(config, CONFIG_SCHEMA);
  }
};
Provide feedback to improve future suggestions

Nice Catch Incorrect Not in Scope Not in coding standard Other

💬 Looking for more details? Reply to this comment to chat with Korbit.

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