Skip to content

Extracts Saml::XML into SamlIdp::XML to free up the use of the Saml namespace#210

Open
pelted wants to merge 5 commits intosaml-idp:masterfrom
kolide:collapse-saml-module
Open

Extracts Saml::XML into SamlIdp::XML to free up the use of the Saml namespace#210
pelted wants to merge 5 commits intosaml-idp:masterfrom
kolide:collapse-saml-module

Conversation

@pelted
Copy link
Contributor

@pelted pelted commented Jul 31, 2024

What this PR does:

  • moves XML module out of the Saml namespace and into SamlIdp 1
  • updates references from Saml::XML to SamlIdp::XML

Why

Defining such a generic namespace as Saml in this gem causes namespace collisions for projects attempting to use saml-idp which may define their own Saml module. We shouldn't do that here. There was even a note to that affect in the form of a long standing TODO item to extract this.

Footnotes

  1. This is opinionated to avoid the repetition of SamlIdp::Saml::Xml

@Zogoo
Copy link
Collaborator

Zogoo commented Aug 5, 2024

Thanks for your feedback and nice improvement.

@Zogoo Zogoo self-assigned this Aug 5, 2024
@Zogoo Zogoo self-requested a review August 5, 2024 08:24
@Zogoo Zogoo assigned pelted and unassigned Zogoo Aug 5, 2024
@Zogoo Zogoo requested a review from jphenow August 13, 2024 07:16
@Zogoo
Copy link
Collaborator

Zogoo commented Sep 18, 2024

@pelted I have fixed the flaky test can you update your branches with the latest master branch?

@Zogoo
Copy link
Collaborator

Zogoo commented Nov 1, 2024

@pelted can you update your branch with the latest master branch?

@pelted pelted force-pushed the collapse-saml-module branch from ef8eb05 to f368f01 Compare February 2, 2026 18:38
@pelted
Copy link
Contributor Author

pelted commented Feb 2, 2026

It's been a bit, but I've updated this PR with the latest from master.

@pelted pelted requested a review from Zogoo February 2, 2026 18:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants