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

Add DefaultFunc to repo ruleset required deploy env, base role IDs to docs #1916

Merged
merged 1 commit into from
Sep 22, 2023

Conversation

o-sama
Copy link
Contributor

@o-sama o-sama commented Sep 22, 2023

Resolves #1915


Before the change?

  • If an empty list is passed to rules.required_deployments.required_deployment_environments, the following is what we get:
    required_deployments:[<nil>]
    The reason it's bad is because we want to be able to strictly specify an empty list, the GitHub API accepts that as a valid input.
    The reason it's set to nil is because the elements of rules.required_deployments are pointers, an empty slice is the zero value and thus treated as a nil pointer.

After the change?

  • Setting a DefaultFunc to return an empty slice gives us the following:
    required_deployments:[map[required_deployment_environments:[]]]
    The API accepts this as a valid input and can successfully apply changes. It also doesn't affect anything that's existing since any input overrides the DefaultFunc.
  • Added base role mapping in the docs since there seems to be confusion regarding this.

Pull request checklist

  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been reviewed and added / updated if needed (for bug fixes / features)

Does this introduce a breaking change?

Please see our docs on breaking changes to help!

  • Yes
  • No

@nickfloyd nickfloyd added the Type: Bug Something isn't working as documented label Sep 22, 2023
Copy link
Contributor

@nickfloyd nickfloyd left a comment

Choose a reason for hiding this comment

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

Thanks for catching all of this @o-sama 💥

@nickfloyd nickfloyd added the Type: Documentation Improvements or additions to documentation label Sep 22, 2023
@nickfloyd nickfloyd merged commit d486f73 into integrations:main Sep 22, 2023
7 checks passed
@o-sama
Copy link
Contributor Author

o-sama commented Sep 22, 2023

Thanks man! I'm just fixing things people are finding, couldn't do it without all the issues being raised 🙂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Bug Something isn't working as documented Type: Documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG]: repository_ruleset crash on empty required_deployment_environments
2 participants