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

Migrate from "should" to "chai" #687

Merged
merged 1 commit into from
Apr 8, 2022
Merged

Conversation

kg0r0
Copy link
Contributor

@kg0r0 kg0r0 commented Apr 8, 2022

Description

Fixed #653

I fixed it the same way as node-saml Issue. (node-saml/node-saml#41)
There are some points that have been specially dealt with.

(1) In the following code, "match" was not used. The reason I did not use "match" in the following code is because, as shown in this Issue, if the comparison target is a string, an error like "TypeError: re.exec is not a function" will occur.
https://github.com/node-saml/passport-saml/pull/687/files#diff-855cb92ef30e211592bac8169dcf23d250f18e57f7f662bab42f31136b3794eeR234

(2) The following code does not use "startWith".The reason for this is that the same does not exist in chai as described in this article.
https://github.com/node-saml/passport-saml/pull/687/files#diff-855cb92ef30e211592bac8169dcf23d250f18e57f7f662bab42f31136b3794eeR165

Please let me know if there is anything else that needs to be fixed in addition to the above, so that I can fix it as soon as possible.

Checklist:

  • Issue Addressed: [x]
  • Link to SAML spec: [ ]
  • Tests included? [x]
  • Documentation updated? [ ]

@kg0r0 kg0r0 force-pushed the feat/chai branch 3 times, most recently from e544a73 to 460c7dd Compare April 8, 2022 15:41
@cjbarth cjbarth added the chore Refactoring, etc. to keep code-rot in check. label Apr 8, 2022
@cjbarth cjbarth merged commit 3f76971 into node-saml:master Apr 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chore Refactoring, etc. to keep code-rot in check.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove unmaintained should.js
2 participants