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

Remove further tempstorage dependencies. #689

Merged
merged 2 commits into from
Aug 21, 2019

Conversation

dwt
Copy link
Contributor

@dwt dwt commented Aug 16, 2019

This tries to fix #668 by removing further tempstorage dependencies.

I am pretty sure that just removing tempstorage from the [docs] section of extras require is pretty safe as I can't find any mention of tempstorage in the documentation. I am a bit more hesitant about the requirements.rtd.txt - but am pretty sure that this is also not required.

Feedback welcome.

@dwt dwt requested review from icemac and jugmac00 August 16, 2019 11:20
@jugmac00
Copy link
Member

@dataflake introduced both lines with tempstorage when he worked on #571

Maybe it has to do with documenting the options?
https://zope.readthedocs.io/en/latest/operation.html#zope-configuration-reference

I am not very adept with RTD and Sphinx, so I have to pass on this review.

P.S.: buildout finally works for Zope

  • though I'd still like to know why the tests, which also use buildout, passed but the manual buildout failed. I'd like to have failing tests in future when there is a problem with buildout.
  • the buildout ends with this message

requirements: Running '/home/jugmac00/Tests/Zope/bin/zopepy util.py'
Unused options for requirements: 'update-command'.

I think this has nothing to do with this pull request, though.

@jugmac00 jugmac00 removed their request for review August 17, 2019 16:28
@dwt
Copy link
Contributor Author

dwt commented Aug 19, 2019

I would argue, that the failing build is actually the test here. It tells you that there is a requirement for a package that is installed, which is not version pinned -> thus the build fails. Adding a test that buildout builds fail if a package is used but not version pinned seems to me to be a redundant test of a buildout feature.

@dwt
Copy link
Contributor Author

dwt commented Aug 19, 2019

@jugmac00 Do you have any Idea where in the docs it mentions tempstorage? I couldn't find anything, so it must be without using the actual name - but then again I don't really know the documentation.

@d-maurer
Copy link
Contributor

d-maurer commented Aug 19, 2019 via email

Copy link
Member

@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.

Removing tempstorage from the documentation requirements breaks the RTD build.

@@ -1,4 +1,3 @@
repoze.sphinx.autointerface
Zope
ZConfig==3.5.0
tempstorage
Copy link
Member

Choose a reason for hiding this comment

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

This is still required as long as the ZConfig schema contains the tempstorge declaration.
See https://readthedocs.org/projects/zope/builds/9537827/ for the failed RTD build.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@icemac icemac merged commit b36f4d0 into master Aug 21, 2019
@icemac icemac deleted the fix_buildout_after_tempstorage_removal branch August 21, 2019 05:58
@dataflake dataflake mentioned this pull request Jul 5, 2021
5 tasks
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.

Installation of current head does not work for Python 2.7
4 participants