Skip to content

Conversation

@vkuma17
Copy link

@vkuma17 vkuma17 commented Mar 3, 2025

Description

refactor: da best input practices
Issue

Release required?

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

Following variables were renamed as part of da best input practices

  • tags --> event_notifications_tags
  • event_notification_name --> event_notifications_name
  • existing_en_instance_crn --> existing_event_notifications_instance_crn
  • en_key_ring_name --> event_notifications_key_ring_name
  • en_key_name --> event_notifications_key_name
  • skip_en_kms_auth_policy --> skip_event_notifications_kms_iam_auth_policy
  • skip_en_cos_auth_policy --> skip_event_notifications_cos_iam_auth_policy
  • skip_cos_kms_auth_policy --> skip_cos_kms_iam_auth_policy
  • retention_enabled --> enable_retention
  • skip_en_sm_auth_policy --> skip_event_notifications_secrets_manager_iam_auth_policy
  • cbr_rules --> event_notifications_instance_cbr_rules

Default values were changed for below variables

  • cos_key_name --> event-notifications-cos-key
  • cos_key_ring_name --> event-notifications-cos-key-ring
  • event_notifications_key_name -> event-notifications-key
  • event_notifications_key_ring_name -> event-notifications-key-ring

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.

@vkuma17 vkuma17 requested review from Aashiq-J and Ak-sky as code owners March 3, 2025 04:41
@ocofaigh
Copy link
Contributor

The current variation is getting replaced by the new variation in -> #395

Can I suggest we just make the changes directly on the new variation in that PR @whoffler . @Vipin654 perhaps you can review the PR to ensure it aligns with best practises?

@whoffler
Copy link
Contributor

@Vipin654 @ocofaigh Reading the Review DA inputs to align with best practices epic, I thought we were moving away from abbreviations for services?

Should for example skip_cos_kms_auth_policy above be skip_cloud_object_storage_key_management_service_auth_policy?

@Ak-sky
Copy link
Member

Ak-sky commented May 11, 2025

@ocofaigh, can we close this PR as this Event Notification DA Fully configurable PR is merged now.

@rajatagarwal-ibm
Copy link
Member

This PR should be closed as there's no more standard variation.

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.

5 participants