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

Event orch resource singular names #517

Merged

Conversation

alenapan
Copy link
Contributor

@alenapan alenapan commented Jun 1, 2022

Changes

Schema attributes that represent a list of objects need to have singular names to adhere to the convention. This PR fixes inconsistencies in the following resources:

pagerduty_event_orchestration:

  • integrations -> integration
  • documentation

pagerduty_event_orchestration_router:

  • sets -> set
  • rules -> rule
  • conditions -> condition
  • documentation

pagerduty_event_orchestration_unrouted:

  • sets -> set

  • rules -> rule

  • conditions -> condition

  • actions.variables -> actions.variable

  • actions.extractions -> actions.extraction

  • documentation
    pagerduty_event_orchestration_service:

  • sets -> set

  • rules -> rule

  • conditions -> condition

  • actions.variables -> actions.variable

  • actions.extractions -> actions.extraction

  • actions.pagerduty_automation_actions -> actions.pagerduty_automation_action

  • actions.automation_actions -> actions.automation_action

  • actions.automation_actions.headers -> actions.automation_actions.header

  • actions.automation_actions.parameters -> actions.automation_actions.parameter

  • documentation

Testing

Acceptance Tests:

image

@alenapan alenapan changed the base branch from master to event-orchestrations June 1, 2022 01:48
…ons, pd_automation_actions, automations_action (headers, params) singular
@alenapan alenapan marked this pull request as ready for review June 1, 2022 03:17
Type: schema.TypeList,
Optional: true,
Elem: &schema.Resource{
Schema: eventOrchestrationPathVariablesSchema,
},
},
"extractions": {
"extraction": {
Copy link
Contributor

@alexzakabluk alexzakabluk Jun 1, 2022

Choose a reason for hiding this comment

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

I have checked ruleset example https://github.com/PagerDuty/terraform-provider-pagerduty/blob/master/pagerduty/resource_pagerduty_ruleset_rule.go#L246 and it looks like extractions are kept as plural there. Same with subconditions. Is that overlooked in rulesets?

Copy link
Contributor

Choose a reason for hiding this comment

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

Does it have something to do with level of nesting?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I believe it was either overlooked or added before the naming convention was established because variables that were added later are singular: https://github.com/PagerDuty/terraform-provider-pagerduty/blob/master/pagerduty/resource_pagerduty_ruleset_rule.go#L285

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

@alexzakabluk Yeah, it looks like I overlooked those fields in the resource_pagerduty_ruleset_rule. I've been trying to keep the naming convention of singular object names for objects that repeat in a definition. The reasoning has been to stay consistent with the other objects in the PagerDuty provider. If we were starting from scratch we would probably go with the pluralized names to match the Best Practices link you posted.

Copy link
Contributor

@stmcallister stmcallister left a comment

Choose a reason for hiding this comment

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

Thanks @alenapan !! 👍 🎉 🌮

@stmcallister stmcallister merged commit 4fcf232 into event-orchestrations Jun 1, 2022
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