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

Restored ability to use own explicit version of zodb-temporary-storage. #160

Merged
merged 2 commits into from
Jan 18, 2021

Conversation

mauritsvanrees
Copy link
Sponsor Member

You used to be able to set zodb-temporary-storage = <some config /> and have this config in the site.zcml.
Most important use of this was to let this empty, effectively disabling the temporary storage.
PR #93 changed this so you could do this in a more natural way, by setting zodb-temporary-storage = false.

But this (accidentally) removed the possibility for setting an explicit own version of the temporary storage snippet, and meant the only options now were false or true.
The documentation still mentions:

If given, Zope's default temporary storage definition will be replaced by the lines of this parameter.

You used to be able to set `zodb-temporary-storage = <some config />` and have this config in the `site.zcml`.
Most important use of this was to let this empty, effectively disabling the temporary storage.
PR #93 changed this so you could do this in a more natural way.
by setting `zodb-temporary-storage = false`.

But this (accidentally) removed the possibility for setting an explicit own version of the temporary storage snippet,
and meant the only options now were false or true.
The documentation still mentions:

   If given Zope's default temporary storage definition will be replaced by
   the lines of this parameter.
@mister-roboto

This comment has been minimized.

@mauritsvanrees
Copy link
Sponsor Member Author

Note: no Jenkins jobs needed, because Jenkins does not run our tests.

Copy link

@icemac icemac left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

Should there be a test?

They fail without the fix from the previous commit.
@mauritsvanrees
Copy link
Sponsor Member Author

LGTM.

Should there be a test?

That may be good, yes. I have added two tests now.

@mauritsvanrees mauritsvanrees merged commit d1f8785 into master Jan 18, 2021
@mauritsvanrees mauritsvanrees deleted the maurits/fix-explicit-zodb-temporary-storage branch January 18, 2021 11:31
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.

3 participants