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.

Sign up for GitHub

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

Jump to bottom

Tekton: Improved property type handling #4595

Merged
merged 2 commits into from
Jun 9, 2023
Merged

Tekton: Improved property type handling #4595

merged 2 commits into from
Jun 9, 2023

Conversation

briangleeson
Copy link
Contributor

Description

Add force new to property typefield in Tekton

Community Note

  • Please vote on this pull request by adding a 👍 reaction to the original pull request comment to help the community and maintainers prioritize this request
  • Please do not leave "+1" or other comments that do not add relevant new information or questions, they generate extra noise for pull request followers and do not help prioritize the request

Output from acceptance testing:

$ make testacc TEST=./ibm/service/cdtektonpipeline TESTARGS='-run=TestAccIBMCdTekton*'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./ibm/service/cdtektonpipeline -v -run=TestAccIBMCdTekton* -timeout 700m
=== RUN   TestAccIBMCdTektonPipelineDefinitionDataSourceBasic
--- PASS: TestAccIBMCdTektonPipelineDefinitionDataSourceBasic (90.71s)
=== RUN   TestAccIBMCdTektonPipelinePropertyDataSourceBasic
--- PASS: TestAccIBMCdTektonPipelinePropertyDataSourceBasic (111.26s)
=== RUN   TestAccIBMCdTektonPipelinePropertyDataSourceAllArgs
--- PASS: TestAccIBMCdTektonPipelinePropertyDataSourceAllArgs (72.58s)
=== RUN   TestAccIBMCdTektonPipelineDataSourceBasic
--- PASS: TestAccIBMCdTektonPipelineDataSourceBasic (81.49s)
=== RUN   TestAccIBMCdTektonPipelineDataSourceAllArgs
--- PASS: TestAccIBMCdTektonPipelineDataSourceAllArgs (69.92s)
=== RUN   TestAccIBMCdTektonPipelineTriggerPropertyDataSourceBasic
--- PASS: TestAccIBMCdTektonPipelineTriggerPropertyDataSourceBasic (93.74s)
=== RUN   TestAccIBMCdTektonPipelineTriggerPropertyDataSourceAllArgs
--- PASS: TestAccIBMCdTektonPipelineTriggerPropertyDataSourceAllArgs (105.34s)
=== RUN   TestAccIBMCdTektonPipelineTriggerDataSourceBasic
--- PASS: TestAccIBMCdTektonPipelineTriggerDataSourceBasic (72.15s)
=== RUN   TestAccIBMCdTektonPipelineTriggerDataSourceAllArgs
--- PASS: TestAccIBMCdTektonPipelineTriggerDataSourceAllArgs (88.33s)
=== RUN   TestAccIBMCdTektonPipelineDefinitionBasic
--- PASS: TestAccIBMCdTektonPipelineDefinitionBasic (98.59s)
=== RUN   TestAccIBMCdTektonPipelinePropertyBasic
--- PASS: TestAccIBMCdTektonPipelinePropertyBasic (160.59s)
=== RUN   TestAccIBMCdTektonPipelinePropertyAllArgs
--- PASS: TestAccIBMCdTektonPipelinePropertyAllArgs (137.65s)
=== RUN   TestAccIBMCdTektonPipelineBasic
--- PASS: TestAccIBMCdTektonPipelineBasic (65.97s)
=== RUN   TestAccIBMCdTektonPipelineAllArgs
--- PASS: TestAccIBMCdTektonPipelineAllArgs (198.78s)
=== RUN   TestAccIBMCdTektonPipelineTriggerPropertyBasic
--- PASS: TestAccIBMCdTektonPipelineTriggerPropertyBasic (126.62s)
=== RUN   TestAccIBMCdTektonPipelineTriggerPropertyAllArgs
--- PASS: TestAccIBMCdTektonPipelineTriggerPropertyAllArgs (148.01s)
=== RUN   TestAccIBMCdTektonPipelineTriggerBasic
--- PASS: TestAccIBMCdTektonPipelineTriggerBasic (170.98s)
=== RUN   TestAccIBMCdTektonPipelineTriggerAllArgs
--- PASS: TestAccIBMCdTektonPipelineTriggerAllArgs (145.61s)
PASS
ok      github.com/IBM-Cloud/terraform-provider-ibm/ibm/service/cdtektonpipeline        2039.418s

@briangleeson briangleeson marked this pull request as draft May 30, 2023 16:32
@briangleeson briangleeson force-pushed the propertyTypeUpdate branch 2 times, most recently from 9080c74 to 76668c3 Compare May 30, 2023 17:11
@briangleeson briangleeson marked this pull request as ready for review May 30, 2023 17:12
hkantare
hkantare reviewed May 31, 2023
View reviewed changes
ibm/service/cdtektonpipeline/resource_ibm_cd_tekton_pipeline_trigger_property.go
@@ -278,8 +279,8 @@ func resourceIBMCdTektonPipelineTriggerPropertyUpdate(context context.Context, d
" The resource must be re-created to update this property.", "name"))
}
if d.HasChange("type") {
replaceTektonPipelineTriggerPropertyOptions.SetType(d.Get("type").(string))
Copy link
Collaborator

Choose a reason for hiding this comment

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

Once we mark the attribute as ForceNew true this block of code if d.HasChange("type") {} is not required .

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is generated by the openapi-sdk-generator, so may be a bug in the generator causing it to add this block erroneously

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ok . We will talk to generator team

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will this prevent this PR from being merged unless the block is removed?

@hkantare hkantare merged commit e20578d into IBM-Cloud:master Jun 9, 2023
omaraibrahim pushed a commit to omaraibrahim/terraform-provider-ibm that referenced this pull request Jul 20, 2023
@briangleeson @omaraibrahim
Tekton: Improved property type handling (IBM-Cloud#4595)
cfade49 
* Tekton: Improved property type handling

* chore(tekton): tweaks and package updates
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hkantare hkantare Awaiting requested review from hkantare

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@briangleeson @hkantare