Skip to content

Correct YARD comments to reflect that settings param cannot be nil#790

Open
alstrocrack wants to merge 3 commits into
SAML-Toolkits:masterfrom
alstrocrack:fix-comments
Open

Correct YARD comments to reflect that settings param cannot be nil#790
alstrocrack wants to merge 3 commits into
SAML-Toolkits:masterfrom
alstrocrack:fix-comments

Conversation

@alstrocrack
Copy link
Copy Markdown

@alstrocrack alstrocrack commented May 3, 2026

Status

READY

Migrations

NO

Description

methods of OneLogin::RubySaml::Authrequest have some incorrect YARD comments.
They shouldn't accept settings param as nil because process causes NoMethodError at each settings.idp_sso_service_url part.

Related PRs

N/A

Todos

  • Tests
  • Documentation

Deploy Notes

N/A

Steps to Test or Reproduce

N/A

Impacted Areas in Application

N/A

Note

Claude Code suggested that we could add a validation to ensure these methods don't accept nil, like this:
If this fix seems to be valid, I can add validations like this.

def create(settings, params = {})
  raise SettingError.new "Invalid settings, not nil settings should be provided" if settings.nil?
  ...

@alstrocrack alstrocrack changed the title Fix comments Correct YARD comments to reflect that settings param cannot be nil May 3, 2026
@alstrocrack alstrocrack marked this pull request as ready for review May 3, 2026 08:25
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.

1 participant