Skip to content

Comments

Gizzigg perforce bug fix#864

Open
gizzigg wants to merge 3 commits intomainfrom
gizzigg-perforce-bug-fix
Open

Gizzigg perforce bug fix#864
gizzigg wants to merge 3 commits intomainfrom
gizzigg-perforce-bug-fix

Conversation

@gizzigg
Copy link
Contributor

@gizzigg gizzigg commented Feb 6, 2026

Issue number:

Summary

This PR fixes a bug in the p4-code-review module that was causing it to error out when the config_php_source variable was set to null.

Changes

  • Modified the container_definitions in the p4-code-review module to conditionally include secrets only when config_php_source is not null
  • Added a ternary operator to handle null values gracefully: secrets = var.config_php_source != null ? [...] : []

User experience

Before:
When users attempted to deploy the p4-code-review module without specifying a config_php_source value (leaving it as null), the deployment would fail with an error due to the module trying to access the valueFrom property of a null value.

After:
Users can now deploy the p4-code-review module with config_php_source set to null without encountering errors. The module gracefully handles the null value and does not attempt to access properties of a null value.

Checklist

If your change doesn't seem to apply, please leave them unchecked.

  • [ x] I have performed a self-review of this change
  • [ x] Changes have been tested
  • Changes are documented
Is this a breaking change? no

Acknowledgment

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Disclaimer: We value your time and bandwidth. As such, any pull requests created might not be successful.

Copy link
Contributor

@danielw-aws danielw-aws left a comment

Choose a reason for hiding this comment

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

LGTM

@hwkiem
Copy link
Contributor

hwkiem commented Feb 6, 2026

@gabebatista

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

3 participants