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

Partial attributes to security settings doesn't nullify missing ones #465

Merged

Conversation

davidlibrera
Copy link
Contributor

@davidlibrera davidlibrera commented Jul 3, 2018

I supposed this was a bug, because if I want to enable metadata signing I need to provide all attributes, while I just want to enable it without change other attributes.

@pitbulk
Copy link
Collaborator

pitbulk commented Jul 3, 2018

I think you are right and the current behavior can confuse the developers.
The problem right now with that PR is that its break current integrations that expect the old behavior.

In order to maintain compatibility, and reduce confusion, what do you think if we add a second parameter, by default the old behavior, to control when or not respect security values?

@davidlibrera davidlibrera force-pushed the bug/security-attributes-override branch from 11cecaa to b37366e Compare July 3, 2018 17:31
@davidlibrera
Copy link
Contributor Author

Great! I changed the method with b37366e and I added another example in order to test both behaviours.

Do you agree?

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.

None yet

2 participants