Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix #655: Test Env map make mutable #656

Merged
merged 1 commit into from
Mar 21, 2024

Conversation

melloware
Copy link
Contributor

Fix #655: All maps mutable

@ia3andy
Copy link
Collaborator

ia3andy commented Mar 21, 2024

@melloware this is not what I meant, Immutable Map are ok, we just need to make sure we don't mutate map from we don't know the origin. So whenever a put is made from a Map created elsewhere, we need to create a new Map from it.

@melloware
Copy link
Contributor Author

@ia3andy i figured it was better to be safe then sorry. But yeah to me it looks like its only one line as testenv comes from a Config property.

PR updated

@melloware
Copy link
Contributor Author

Test env comes from here

    @ConfigDocDefault("CI=true")
    Map<String, String> testEnv();

so it must be an immutable map being passed from Config

@melloware melloware changed the title Fix #655: All maps mutable Fix #655: Test Env map make mutable Mar 21, 2024
Copy link
Collaborator

@ia3andy ia3andy left a comment

Choose a reason for hiding this comment

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

LGTM

@melloware melloware merged commit f54543d into quarkiverse:main Mar 21, 2024
2 checks passed
@melloware melloware deleted the mutable-maps branch March 21, 2024 14:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Maps: Make all maps mutable
2 participants