Skip to content

Conversation

knee-na
Copy link

@knee-na knee-na commented Aug 25, 2025

Description

Release required?

  • No release
  • Patch release (x.x.X)
  • Minor release (x.X.x)
  • Major release (X.x.x)
Release notes content

Run the pipeline

If the CI pipeline doesn't run when you create the PR, the PR requires a user with GitHub collaborators access to run the pipeline.

Run the CI pipeline when the PR is ready for review and you expect tests to pass. Add a comment to the PR with the following text:

/run pipeline

Checklist for reviewers

  • If relevant, a test for the change is included or updated with this PR.
  • If relevant, documentation for the change is included or updated with this PR.

For mergers

  • Use a conventional commit message to set the release level. Follow the guidelines.
  • Include information that users need to know about the PR in the commit message. The commit message becomes part of the GitHub release notes.
  • Use the Squash and merge option.

@knee-na knee-na marked this pull request as ready for review August 25, 2025 19:12
@knee-na knee-na requested review from jor2 and vbontempi as code owners August 25, 2025 19:12
ibm_catalog.json Outdated
"title": "Supports building, deploying and running Jakarta EE/MicroProfile applications on Liberty in public cloud",
"description": "Supports building, deploying and running Jakarta EE/MicroProfile applications on Liberty in public cloud.<br/>For more details about the features and the options available please refer to this [page](https://github.com/terraform-ibm-modules/terraform-ibm-enterprise-app-java/blob/main/solutions/fully-configurable/DA-details.md)."
"title": "Creates an Enterprise Application Service instance",
"description": "Get started with Enterprise Application Service by creating an instance. [Learn more](https://github.com/terraform-ibm-modules/terraform-ibm-enterprise-app-java/blob/main/solutions/fully-configurable/DA-details.md)."
Copy link
Member

Choose a reason for hiding this comment

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

I took the description from the Enterprise application service description, I think it provides a better idea to the user of what is going to deploy

Copy link
Author

Choose a reason for hiding this comment

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

Updated

Copy link
Member

@vbontempi vbontempi left a comment

Choose a reason for hiding this comment

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

some comments

variables.tf Outdated
validation {
condition = var.repos_git_token != null ? (var.source_repo != null && var.config_repo != null) : (var.source_repo == null && var.config_repo == null)
error_message = "If at least one of var.source_repo, var.config_repo, var.repos_git_token input parameters is not null all of them must be assigned with a value, but var.repos_git_token is null."
error_message = "`var.repos_git_token` cannot be set to `null` if `var.source_repo` and `var.config_repo` are not also set to `null`."
Copy link
Member

Choose a reason for hiding this comment

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

Still not fully clear (also the original one probably): we need to underline that these parameters must be all set to null or all set to a value != null (this means that if any of them is != null, also the other ones mut be != null)

Copy link
Author

Choose a reason for hiding this comment

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

Updated

Copy link
Author

Choose a reason for hiding this comment

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

Would we also throw this error if var.repos_git_token is set to a value but one or both of the other variables are set to null ?

Copy link
Member

Choose a reason for hiding this comment

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

yes

Copy link
Author

Choose a reason for hiding this comment

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

Updated

@knee-na knee-na requested a review from vbontempi August 26, 2025 15:22
@vbontempi
Copy link
Member

/run pipeline

@vbontempi
Copy link
Member

@maheshwarishikha adding you as reviewer, I contributed on catalog content

@vbontempi
Copy link
Member

tests to be fixed through #114 (rebase once merged that)

vbontempi
vbontempi previously approved these changes Sep 5, 2025
Copy link
Member

@vbontempi vbontempi left a comment

Choose a reason for hiding this comment

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

ok

@vbontempi
Copy link
Member

/run pipeline

@vbontempi
Copy link
Member

/run pipeline

@vbontempi
Copy link
Member

/run pipeline

jor2
jor2 previously approved these changes Oct 2, 2025
@jor2 jor2 self-requested a review October 2, 2025 12:29
Copy link
Member

@jor2 jor2 left a comment

Choose a reason for hiding this comment

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

need to rebase

@vbontempi
Copy link
Member

need to rebase

@knee-na ^^

@vbontempi
Copy link
Member

/run pipeline

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.

3 participants