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 missing valid custom role permissions #1023

Merged
merged 5 commits into from
Mar 22, 2022
Merged

Add missing valid custom role permissions #1023

merged 5 commits into from
Mar 22, 2022

Conversation

lucascantor
Copy link
Contributor

@lucascantor lucascantor marked this pull request as ready for review March 15, 2022 22:19
@lucascantor
Copy link
Contributor Author

Thanks for your patience with my first PR in the terraform-provider-okta repo. I took a look at the contributing guidelines but don't see anything referencing validCustomRolePermissions in the existing code or tests.

That said, if I'm missing anything, please let me know, and I'll do my best to add it 🙏🏼

@monde
Copy link
Collaborator

monde commented Mar 16, 2022

Thank you @lucascantor !

@monde monde self-requested a review March 16, 2022 00:07
@lucascantor
Copy link
Contributor Author

@monde I see I need approval before 1 workflow will run, as I'm a first-time contributor:

Screen Shot 2022-03-22 at 12 04 33 PM

@monde
Copy link
Collaborator

monde commented Mar 22, 2022

Hi @lucascantor , apologies for my slowness. Can you make strings in the var validCustomRolePermissions be alphabetized and have each element on a new line? I did something similar here on validEmailTemplateTypes https://github.com/okta/terraform-provider-okta/pull/1012/files

I think the workflow will fail when when we have it run on your cloned repo. I'll have to double check after the change, and might have to start a new branch and cherry pick in your commits.

@lucascantor
Copy link
Contributor Author

@monde not a problem at all. I sincerely appreciate the help.

I believe I've made the change you're looking for now 👍🏼

Copy link
Collaborator

@monde monde left a comment

Choose a reason for hiding this comment

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

Looks good, will be easy to see changes in the future, thank you @lucascantor

@monde monde merged commit ac62cff into okta:master Mar 22, 2022
@monde
Copy link
Collaborator

monde commented Mar 22, 2022

I'll get this into the next release @lucascantor , thank you!

@lucascantor lucascantor deleted the patch-1 branch March 22, 2022 22:42
@monde
Copy link
Collaborator

monde commented Apr 8, 2022

@lucascantor this is released as v3.23.0

@lucascantor
Copy link
Contributor Author

@monde I noticed and we're already using it - thanks so much!

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.

2 participants