Skip to content

[Bug] Fix maven setting update#4261

Merged
wolfboys merged 1 commit intoapache:devfrom
Gianzie:fix_mvn_update
Jun 21, 2025
Merged

[Bug] Fix maven setting update#4261
wolfboys merged 1 commit intoapache:devfrom
Gianzie:fix_mvn_update

Conversation

@Gianzie
Copy link
Copy Markdown
Contributor

@Gianzie Gianzie commented Jun 17, 2025

What changes were proposed in this pull request

Update "Map<String, Setting> SETTINGS = new ConcurrentHashMap<>();" first, then "getMavenConfig().updateConfig();"

Brief change log

Verifying this change

This change is a trivial rework / code cleanup without any test coverage.

(or)

This change is already covered by existing tests, such as (please describe tests).

(or)

This change added tests and can be verified as follows:

Does this pull request potentially affect one of the following parts

  • Dependencies (does it add or upgrade a dependency): (yes / no)

@sonarqubecloud
Copy link
Copy Markdown

@wolfboys wolfboys requested a review from Copilot June 21, 2025 14:17
Copy link
Copy Markdown
Contributor

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 pull request fixes a bug by reordering the update of the Maven configuration so that the in-memory SETTINGS map is updated before calling getMavenConfig().updateConfig().

  • The updateConfig() call was moved to after updating the SETTINGS map.
  • The ordering change ensures that the Maven configuration reflects the latest setting changes.

Optional<Setting> optional = Optional.ofNullable(SETTINGS.get(setting.getSettingKey()));
optional.ifPresent(x -> x.setSettingValue(value));

getMavenConfig().updateConfig();
Copy link

Copilot AI Jun 21, 2025

Choose a reason for hiding this comment

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

Moving getMavenConfig().updateConfig() to after the SETTINGS update ensures that the Maven configuration is updated with the latest settings, which resolves the reported bug.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Member

@wolfboys wolfboys left a comment

Choose a reason for hiding this comment

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

LGTM

@wolfboys wolfboys merged commit 5acca1d into apache:dev Jun 21, 2025
42 checks passed
@Gianzie Gianzie deleted the fix_mvn_update branch June 30, 2025 08:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants