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

Replace CurationConcerns::NullLogger with ActiveFedora::NullLogger #1007

Open
awead opened this issue Sep 22, 2016 · 7 comments
Open

Replace CurationConcerns::NullLogger with ActiveFedora::NullLogger #1007

awead opened this issue Sep 22, 2016 · 7 comments

Comments

@awead
Copy link
Contributor

awead commented Sep 22, 2016

There's a PR in on ActiveFedora to use NullLogger, samvera/active_fedora#1153, which is the same as Curation Concerns' null logging object. Once that PR is merged, we can just use that one instead.

@botimer
Copy link

botimer commented Oct 19, 2016

The AF PR is merged, but there is no release containing it yet. Also, it is not clear when it is safe to bump the gemspec dependency to 11.0.x. It's currently '>= 10.3.0.rc1'.

I'm happy to tidy this up with some advice on the versioning.

@awead
Copy link
Contributor Author

awead commented Oct 19, 2016

I think all we need to do is cut a new release of AF.

@jcoyne
Copy link
Contributor

jcoyne commented Oct 20, 2016

If we require AF 11, you'll transitively require rdf 2, which may be a problem for some users

@awead
Copy link
Contributor Author

awead commented Oct 20, 2016

I can backport the NullLogger to version 10, if needed.

@botimer
Copy link

botimer commented Oct 20, 2016

Got it. Since the point is that the logger does nothing, I'd be inclined to leave it in place and close this. Not too much value in binding across gems here when there's no real risk of desync or duplication. And there's a downside in dangling issues.

@awead
Copy link
Contributor Author

awead commented Oct 20, 2016

meh. I'm inclined to backport because it'd be nice to not have the duplication, alternatively, we could tag this for a version of CC that requires AF 11.

@botimer
Copy link

botimer commented Oct 20, 2016

Right. I'm not really invested in any of these approaches -- it's just not the obvious housekeeping thing it seemed to be. A release of AF is needed to address it or a decision made that it's okay as-is.

I'm happy to port/PR to AF 10.3-stable and bump the CC gemspec if that's the winner. Up to you two.

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

No branches or pull requests

3 participants