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

Update secret scope validation to include / characters #892

Merged
merged 2 commits into from
Nov 4, 2021
Merged

Update secret scope validation to include / characters #892

merged 2 commits into from
Nov 4, 2021

Conversation

tylangesmith
Copy link
Contributor

This PR fixes issue #888

nfx
nfx requested changes Nov 2, 2021
View reviewed changes
Copy link
Contributor

@nfx nfx left a comment

Choose a reason for hiding this comment

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

Please remove redundant unit tests and keep only one for creating. Url encoding and validation is the test focus, not already tested functionality.

access/resource_secret_acl_test.go Outdated Show resolved Hide resolved
access/resource_secret_acl_test.go Outdated Show resolved Hide resolved
access/resource_secret_acl_test.go Outdated Show resolved Hide resolved
access/resource_secret_acl_test.go Show resolved Hide resolved
@tylangesmith tylangesmith requested a review from nfx November 2, 2021 22:35
nfx
nfx approved these changes Nov 3, 2021
View reviewed changes
Copy link
Contributor

@nfx nfx left a comment

Choose a reason for hiding this comment

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

Lgtm

@codecov
Copy link

codecov bot commented Nov 3, 2021 •
edited
Loading

Codecov Report

Merging #892 (42aa2d3) into master (5fe12a4) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #892   +/-   ##
=======================================
  Coverage   83.93%   83.93%           
=======================================
  Files          94       94           
  Lines        8567     8567           
=======================================
  Hits         7191     7191           
  Misses        864      864           
  Partials      512      512
Impacted Files Coverage Δ
access/resource_secret_scope.go 87.50% <ø> (ø)

@nfx
Copy link
Contributor

nfx commented Nov 3, 2021

@tylangesmith please rebase onto latest master

@nfx nfx linked an issue Nov 3, 2021 that may be closed by this pull request
[ISSUE] databricks_secret_acl scope property validation does not allow / characters #888
Closed
@tylangesmith
Copy link
Contributor Author

Forgot to tag you this morning @nfx but I believe that should be rebased now

@nfx nfx merged commit 9921c41 into databricks:master Nov 4, 2021
@nfx nfx mentioned this pull request Nov 10, 2021
Merged
michael-berk pushed a commit to michael-berk/terraform-provider-databricks that referenced this pull request Feb 15, 2023
@tylangesmith
Update secret scope validation to include / characters (databricks#892
9d318ea 
)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nfx nfx nfx approved these changes

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

Successfully merging this pull request may close these issues.

[ISSUE] databricks_secret_acl scope property validation does not allow / characters
2 participants
@tylangesmith @nfx