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

Deprecate ?demostorage in favour of demo:// #33

Merged
merged 2 commits into from
May 29, 2023

Conversation

navytux
Copy link
Contributor

@navytux navytux commented May 24, 2023

In 2021 in 2f02bc9 (demo: URI resolver) we added support for generic demo:// URI scheme which allows combining any two storages into a DemoStorage. This support is more generic because e.g. previously ?demostorage was provided only by file:// and zeo:// URI schemes, with the "changes" part there being hardcoded to in-memory MappingStorage.

Now there is no need to use ?demostorage option as we can use demo:// uniformly.

Deprecating ?demostorage in favour of dedicated DemoStorage resolver was actually suggested several times in the past:

#25 (comment)
#25 (comment)
#25 (comment)

/cc @azmeuk, @stevepiercy

In 2021 in 2f02bc9 (demo: URI resolver) we added support for generic
demo:// URI scheme which allows combining any two storages into a
DemoStorage. This support is more generic because e.g. previously
?demostorage was provided only by file:// and zeo:// URI schemes, with
the "changes" part there being hardcoded to in-memory MappingStorage.

Now there is no need to use ?demostorage option as we can use demo://
uniformly.

Deprecating ?demostorage in favour of dedicated DemoStorage resolver was
actually suggested several times in the past:

Pylons#25 (comment)
Pylons#25 (comment)
Pylons#25 (comment)
@navytux
Copy link
Contributor Author

navytux commented May 24, 2023

P.S. @azmeuk, I appologize for my silence on #32.

Copy link
Member

@stevepiercy stevepiercy left a comment

Choose a reason for hiding this comment

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

Docs like good to me. A maintainer should review the source code.

@navytux
Copy link
Contributor Author

navytux commented May 24, 2023

Thanks, @stevepiercy.

In the past I was granted zodburi maintaintership together with @azmeuk, so I think it should be enough if Éloi reviews this.

I would be glad to be proven wrong, but in my experience Jim, Tres and Chris are no longer active in around ZODB.

@navytux navytux requested a review from azmeuk May 24, 2023 11:00
@tseaver suggests at Pylons#33 (review) :

    Rather than removing the docs, please flag this option as deprecated (likewise below).

-> Restore documentation and add

   (deprecated in favour of ``demo:`` URI scheme)

notice.
@navytux
Copy link
Contributor Author

navytux commented May 25, 2023

Thanks, @tseaver.

@navytux
Copy link
Contributor Author

navytux commented May 25, 2023

With 2 approvals I think it is ok to merge this patch. I plan to do so in a day or two.

@navytux navytux merged commit 1736338 into Pylons:master May 29, 2023
6 checks passed
@navytux navytux deleted the y/demostorage-to-demo branch May 29, 2023 08:20
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

Successfully merging this pull request may close these issues.

None yet

3 participants