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

3.0.0 Breaking changes not specified either in Changelog or release notes #597

Closed
christian-hawk opened this issue May 20, 2021 · 10 comments
Labels

Comments

@christian-hawk
Copy link

christian-hawk commented May 20, 2021

On release notes:

This release has some breaking changes and some significant refactors. Please read the CHANGELOG.md carefully to note what few things may need to change in your code before taking this version.

On CHANGELOG.md:
image

There is currently no information about v3.0.0 or about any Breaking Change in CHANGELOG.

@cjbarth
Copy link
Collaborator

cjbarth commented May 20, 2021

I'd say this is a bug. I just want over to see what could be the problem and found it is a bug in our changelog generating tool: github-tools/github-release-notes#116.

@christian-hawk
Copy link
Author

I'd say this is a bug. I just want over to see what could be the problem and found it is a bug in our changelog generating tool: github-tools/github-release-notes#116.

I see @cjbarth It looks like an old one.
Meanwhile is it possible to get a small explanation of what are the breaking changes from 2.x to 3.x?

Lemme know if I can help somehow.

Thanks

@serhalp
Copy link

serhalp commented May 25, 2021

It seems that the 3.0.0 release notes are just incorrectly called "2.2.0". So the release notes are the ones in your screenshot.

@christian-hawk
Copy link
Author

Thanks @serhalp . It seems or are you sure about it?

So as breaking change we have

  1. Not supporting anymore deprecated privateCert. Ok so if we use privateCert, code will break. Pretty clear.
  2. Node-saml separation. About this, is not clear. When would code break? I never found myself using node-saml explicitly when coding.

Thanks

@cjbarth
Copy link
Collaborator

cjbarth commented May 31, 2021

I've created a PR to fix the changelog: #605

@cjbarth
Copy link
Collaborator

cjbarth commented May 31, 2021

@christian-hawk The node-saml separation work shouldn't have caused any breaking changes, but it was a major refactor and a lot of code moved, so there is a strong potential for breakage if you depended on anything but the strict passport-saml API.

@christian-hawk
Copy link
Author

@christian-hawk The node-saml separation work shouldn't have caused any breaking changes, but it was a major refactor and a lot of code moved, so there is a strong potential for breakage if you depended on anything but the strict passport-saml API.

@cjbarth thanks for that. Maybe is not a breaking change, then? Anyway would be nice to have the information you just provided in CHANGELOG.md

@christian-hawk
Copy link
Author

@cjbarth maybe also add to breaking change changelog information that cert is required:

 1) Test SP Meta Helper
   generate meta test
     should generate metafile for provider in idp-metadata folder:
 TypeError: cert is required
  at Object.assertRequired (node_modules/passport-saml/lib/node-saml/utility.js:7:15)
  at SAML.initialize (node_modules/passport-saml/lib/node-saml/saml.js:110:29)
  at new SAML (node_modules/passport-saml/lib/node-saml/saml.js:79:29)
  at new AbstractStrategy (node_modules/passport-saml/lib/passport-saml/strategy.js:26:26)
  at new Strategy (node_modules/passport-saml/lib/passport-saml/strategy.js:154:1)
  at Context.<anonymous> (test/sp-meta.test.js:38:37)
  at processImmediate (internal/timers.js:461:21)

@cjbarth
Copy link
Collaborator

cjbarth commented Jun 4, 2021

@christian-hawk Please look at the PR referenced above for the corrected changelog.

@christian-hawk
Copy link
Author

thanks @cjbarth

@cjbarth cjbarth closed this as completed Jun 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants