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 validators and refactor soft parameter (Splited from 197) #235

Merged
merged 5 commits into from
May 26, 2015

Conversation

pitbulk
Copy link
Collaborator

@pitbulk pitbulk commented May 12, 2015

  • Avoid the use of the soft parameter in each validator.
  • Refact validations.
  • Add test.

…ft parameter from validators. Created the ivar soft that is initialized with the settings.soft. Removed the validate! method
@pitbulk
Copy link
Collaborator Author

pitbulk commented May 12, 2015

Re-factorized the soft parameters of validators on the Logoutresponse class.
This was suggested here: https://github.com/onelogin/ruby-saml/pull/197/files#r27402771

@Lordnibbler, @luisvm Please validate this so I can do the same on Response and LogooutRequest classes.

@pitbulk pitbulk changed the title Split 197. Add validators and refactor soft parameter Add validators and refactor soft parameter (Splited from 197) May 12, 2015
@Lordnibbler
Copy link
Contributor

👍

@pitbulk
Copy link
Collaborator Author

pitbulk commented May 13, 2015

@phlipper this change is what you suggested at #197 (comment) ??

@phlipper
Copy link
Contributor

@pitbulk this looks like a nice improvement, thanks!

def valid_response(opts = {})
opts = default_response_opts.merge!(opts)
def valid_logout_response_document(opts = {})
opts = default_logout_response_opts.merge!(opts)
Copy link
Contributor

Choose a reason for hiding this comment

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

do we really want to alter default_logout_response_opts? it's being modified when using merge!

@luisvm
Copy link
Contributor

luisvm commented May 26, 2015

👍 lgtm!

pitbulk added a commit that referenced this pull request May 26, 2015
Add validators and refactor soft parameter (Splited from 197)
@pitbulk pitbulk merged commit e10328a into master May 26, 2015
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.

4 participants