Skip to content

fix(config.ts): fix config priority - #275

Open
hydrotho wants to merge 2 commits into
di-sukharev:devfrom
hydrotho:fix/config-priority
Open

hydrotho wants to merge 2 commits into
di-sukharev:devfrom
hydrotho:fix/config-priority

Conversation

@hydrotho

Copy link
Copy Markdown

Problem Description

In this project, according to the README documentation, local configuration should have a higher priority over the global configuration. However, in practice, it was observed that the local config wasn't overriding the global config as expected. Upon investigation, I identified that the issue originated from a logical error in the getConfig() function in the src/commands/config.ts file.

Changes

To address this issue, I have made necessary modifications to the getConfig() function. These changes ensure that the local config correctly overrides the global config.

Testing and Validation

To validate the effectiveness of this fix, I conducted a series of tests including various combinations of global and local configurations. These tests have confirmed that the modified code now aligns with the expected behavior as described in our README.

The getConfig function in config.ts has been updated to properly handle environment variables.
@github-actions

Copy link
Copy Markdown

Stale pull request message

@github-actions github-actions Bot closed this Jan 23, 2024
@di-sukharev di-sukharev reopened this Feb 28, 2024
@di-sukharev
di-sukharev changed the base branch from master to dev March 18, 2024 04:41
@di-sukharev

Copy link
Copy Markdown
Owner

hi @hydrotho great job! thank you! could you solve the hanging conflict and i merge it

@di-sukharev

Copy link
Copy Markdown
Owner

should be fixed here there was a refactoring #391, please fix conflicts

@di-sukharev

di-sukharev commented Aug 20, 2026

Copy link
Copy Markdown
Owner

Checked the current master: environment configuration already explicitly takes precedence over ~/.opencommit through mergeConfigs(envConfig, globalConfig), including false and null, and unit tests cover it. This PR conflicts and no longer provides a needed change. I recommend closing it as superseded.

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