[MRESOLVER-126] support conversion from String to Map for config

[kwin]

properties

[michael-o]

This feels like pain. When you have code, you can pass a map, when you are at the command line, it will make the input unreadable. I really fail to see how somenoe will pass Java properties format via a single CLI.

[kwin]

At least for single entries this is good enough also to pass from CLI, e.g. -Daether.connector.http.headers.myrepo="custom-auth-header=customauthvalue". Multiple entries are a pain indeed, but should rarely occur. In any case this is better than before, IMHO. I cannot think of a good solution for multiple entries for CLI or do you have any proposal?

[michael-o]

The only thing I can image is that String will contain a local path to a proprties file. But I really would put this issue aside until someone really complains that this does not work.

[jira-importer]

Resolve #859

[jira-importer]

Resolve #859

[gnodet]

AI Review — PR #63: [MRESOLVER-126] support conversion from String to Map

Hi @kwin, thanks for working on this — the ability to set map-valued config properties from the CLI addresses a real usability gap!

However, this PR is now 5+ years old (opened June 2020) and has accumulated several issues:

🔴 Merge conflicts (high)

The PR is in a CONFLICTING merge state. One of the three target files (src/site/markdown/configuration.md) has been completely removed from master. ConfigUtils.java has also been substantially reformatted and refactored since 2020.

🔴 No CI verification (high)

No CI checks have ever run on this branch. There's no automated verification that the code compiles or passes tests against current master.

⚠️ Unresolved design concern from maintainer (medium)

@michael-o expressed skepticism about parsing Java Properties format from a CLI system property: "This feels like pain... I really fail to see how someone will pass Java properties format via a single CLI." The discussion ended without consensus, with michael-o suggesting the issue be put aside "until someone really complains."

⚠️ Silent exception swallowing (medium)

The catch (IOException e) { // try next key } silently swallows the exception without logging. While StringReader is unlikely to throw IOException in practice, at minimum a TRACE/DEBUG log would aid maintainability.

💡 Properties vs HashMap return type (low)

The method returns a Properties object (which extends Hashtable) from a method declared to return Map<?, ?>. Callers get a synchronized Hashtable-backed instance rather than a plain HashMap. Converting to new HashMap<>(props) before returning would be more consistent.

Recommendation: Given the age, merge conflicts, missing target file, and unresolved design discussion, this PR would need a fresh start. If the feature is still desired, a new PR rebased on current master with a CLI-friendlier format (e.g., key=value,key2=value2) might address the maintainer's concerns better.

---

🤖 This review was generated by ForgeBot using a maker/checker pattern (reviewer + independent verifier). 1 of 6 original findings was removed as a false positive after verification (project compiles at Java 8, not 21+).