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

Merge for release 2.3 #77

Merged
merged 34 commits into from
Sep 14, 2016
Merged

Merge for release 2.3 #77

merged 34 commits into from
Sep 14, 2016

Conversation

DRvanR
Copy link
Contributor

@DRvanR DRvanR commented Sep 13, 2016

No description provided.

thijskh and others added 30 commits February 23, 2016 16:19
…xplicit

The regexp attribute of a Scope used to default to null when not set
explicitly.

According to the spec, when omitted this should be interpreted as it being
false. Therefore, I think the null default is not correct to have.

Also, we should always output an explicit regexp attribute,
as recommended by the spec for document signing reasons:
https://wiki.shibboleth.net/confluence/display/SC/ShibMetaExt+V1.0
Especially relevant now that a HHVM bug causes long hangs while
HHVM is an allowed failure.
shibmd:scope regexp attribute should default to false and be always explicit
Also update tests to use expected key format.

Fixes #62
SimpleSAML_Logger has been refactored to use namespaces. Replace all calls to the old class with the new.
@DRvanR DRvanR added this to the v2.3 milestone Sep 13, 2016
@DRvanR
Copy link
Contributor Author

DRvanR commented Sep 13, 2016

@thijskh @jaimeperez seeing you two did the work here 😉 are there any commits that should not be merged for release 2.3?

@coveralls
Copy link

Coverage Status

Coverage increased (+8.09%) to 63.089% when pulling eeefb67 on master into 34276aa on release-2.x.

@thijskh
Copy link
Member

thijskh commented Sep 13, 2016

I'm wondering if we should merge #69 also.

@jaimeperez
Copy link
Member

The only commit of those I perpetrated that could be a bit more problematic is this one, since it bumps the minimum version of SimpleSAMLphp required by the compat layer.

@thijskh
Copy link
Member

thijskh commented Sep 13, 2016

Otherwise I'm 👍

Add deprecation tags to classes and methods that work with certificate fingerprints.
@coveralls
Copy link

Coverage Status

Coverage increased (+8.09%) to 63.089% when pulling 1cea30c on master into 34276aa on release-2.x.

@DRvanR
Copy link
Contributor Author

DRvanR commented Sep 14, 2016

#69 can and should be merged i.m.o. - a deprecation notice is still fully bc and just informative to users about what functionality will be removed at some point.

The SSP logger is indeed technically a BC break, but only if you use the provided container with SSP which in and of itself is not even a requirement - seems that can and should be fixed with documentation and perhaps an additional interface (for dependecy inversion). Not sure whether or not breaking something that doesn't work out of the box and won't work without knowledge about SSP itself is considered a BC break...

I'm actually in favor of keeping both in as neither should be an issue. Thoughts?

@thijskh
Copy link
Member

thijskh commented Sep 14, 2016

Yes, agreed to add/keep both in.

@jaimeperez
Copy link
Member

I'm assuming both you and us are the main users of this library. So if you don't see a problem with keeping both, and I don't see it either... go ahead! 😉

@DRvanR
Copy link
Contributor Author

DRvanR commented Sep 14, 2016

Awesome, thanks!

@DRvanR DRvanR merged commit 4853f1e into release-2.x Sep 14, 2016
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 22, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants