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

Bump mkdocs and all deps to most recent versions #388

Merged
merged 2 commits into from
Oct 24, 2023
Merged

Conversation

suricactus
Copy link
Collaborator

@suricactus suricactus commented Oct 24, 2023

I have pip freezeed all deps, IMO it is better to have a more precise snapshot on all the deps rather than just some of them.

@m-kuhn
Copy link
Member

m-kuhn commented Oct 24, 2023

@why-not-try-calmer dare having a look?

@why-not-try-calmer
Copy link
Contributor

On it

@@ -2,3 +2,5 @@
template: home.html
title: QField Ecosystem Documentation
---

# QField Ecosystem Documentation
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

NOTE if this is an empty file, it blows up for unknown reason. Decided to reuse a string that is already translated as a workaround.

Comment on lines -127 to +128
emoji_index: !!python/name:materialx.emoji.twemoji
emoji_generator: !!python/name:materialx.emoji.to_svg
emoji_index: !!python/name:material.extensions.emoji.twemoji
emoji_generator: !!python/name:material.extensions.emoji.to_svg
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@why-not-try-calmer
Copy link
Contributor

I have pip freezeed all deps, IMO it is better to have a more precise snapshot on all the deps rather than just some of them.

pre-commit was not freezed. Anything else?

@suricactus
Copy link
Collaborator Author

I did not freeze the dev dependencies, IMO we should not do that?

@why-not-try-calmer
Copy link
Contributor

why-not-try-calmer commented Oct 24, 2023

Beside that it builds fine,

I did not freeze the dev dependencies, IMO we should not do that?

So which dependencies where frozen?

@why-not-try-calmer
Copy link
Contributor

why-not-try-calmer commented Oct 24, 2023

Oh I see you mean that you computed all transitive dependencies from the dependencies enumerated in requirements.txt and pinned them. I believe this makes the build step depend on our pins instead of the versions preferred by the top-level (non-transitive) dependencies. Is that correct? If so what happens if, for example, the maintainers of mkdocs want to change the version of click from A to B, where A is the one you pinned and B is the one they prefer?

For reference, non-transitive / top-level depencies are these:

CairoSVG==2.7.1
  - cairocffi [required: Any, installed: 1.6.1]
    - cffi [required: >=1.1.0, installed: 1.16.0]
      - pycparser [required: Any, installed: 2.21]
  - cssselect2 [required: Any, installed: 0.7.0]
    - tinycss2 [required: Any, installed: 1.2.1]
      - webencodings [required: >=0.4, installed: 0.5.1]
    - webencodings [required: Any, installed: 0.5.1]
  - defusedxml [required: Any, installed: 0.7.1]
  - pillow [required: Any, installed: 10.1.0]
  - tinycss2 [required: Any, installed: 1.2.1]
    - webencodings [required: >=0.4, installed: 0.5.1]
cryptography==41.0.4
  - cffi [required: >=1.12, installed: 1.16.0]
    - pycparser [required: Any, installed: 2.21]
fancyboxmd==1.1.0
  - markdown [required: >=3.0, installed: 3.5]
mkdocs-material==9.4.6
  - babel [required: ~=2.10, installed: 2.13.0]
  - colorama [required: ~=0.4, installed: 0.4.6]
  - jinja2 [required: ~=3.0, installed: 3.1.2]
    - MarkupSafe [required: >=2.0, installed: 2.1.3]
  - markdown [required: ~=3.2, installed: 3.5]
  - mkdocs [required: ~=1.5,>=1.5.3, installed: 1.5.3]
    - click [required: >=7.0, installed: 8.1.7]
    - ghp-import [required: >=1.0, installed: 2.1.0]
      - python-dateutil [required: >=2.8.1, installed: 2.8.2]
        - six [required: >=1.5, installed: 1.16.0]
    - jinja2 [required: >=2.11.1, installed: 3.1.2]
      - MarkupSafe [required: >=2.0, installed: 2.1.3]
    - markdown [required: >=3.2.1, installed: 3.5]
    - markupsafe [required: >=2.0.1, installed: 2.1.3]
    - mergedeep [required: >=1.3.4, installed: 1.3.4]
    - packaging [required: >=20.5, installed: 23.2]
    - pathspec [required: >=0.11.1, installed: 0.11.2]
    - platformdirs [required: >=2.2.0, installed: 3.11.0]
    - pyyaml [required: >=5.1, installed: 6.0.1]
    - pyyaml-env-tag [required: >=0.1, installed: 0.1]
      - pyyaml [required: Any, installed: 6.0.1]
    - watchdog [required: >=2.0, installed: 3.0.0]
  - mkdocs-material-extensions [required: ~=1.2, installed: 1.3]
  - paginate [required: ~=0.5, installed: 0.5.6]
  - pygments [required: ~=2.16, installed: 2.16.1]
  - pymdown-extensions [required: ~=10.2, installed: 10.3.1]
    - markdown [required: >=3.2, installed: 3.5]
    - pyyaml [required: Any, installed: 6.0.1]
  - regex [required: >=2022.4, installed: 2023.10.3]
  - requests [required: ~=2.26, installed: 2.31.0]
    - certifi [required: >=2017.4.17, installed: 2023.7.22]
    - charset-normalizer [required: >=2,<4, installed: 3.3.0]
    - idna [required: >=2.5,<4, installed: 3.4]
    - urllib3 [required: >=1.21.1,<3, installed: 2.0.7]
mkdocs-static-i18n==1.1.1
  - mkdocs [required: >=1.5.2, installed: 1.5.3]
    - click [required: >=7.0, installed: 8.1.7]
    - ghp-import [required: >=1.0, installed: 2.1.0]
      - python-dateutil [required: >=2.8.1, installed: 2.8.2]
        - six [required: >=1.5, installed: 1.16.0]
    - jinja2 [required: >=2.11.1, installed: 3.1.2]
      - MarkupSafe [required: >=2.0, installed: 2.1.3]
    - markdown [required: >=3.2.1, installed: 3.5]
    - markupsafe [required: >=2.0.1, installed: 2.1.3]
    - mergedeep [required: >=1.3.4, installed: 1.3.4]
    - packaging [required: >=20.5, installed: 23.2]
    - pathspec [required: >=0.11.1, installed: 0.11.2]
    - platformdirs [required: >=2.2.0, installed: 3.11.0]
    - pyyaml [required: >=5.1, installed: 6.0.1]
    - pyyaml-env-tag [required: >=0.1, installed: 0.1]
      - pyyaml [required: Any, installed: 6.0.1]
    - watchdog [required: >=2.0, installed: 3.0.0]
mkdocs-video==1.5.0
  - lxml [required: >=4.7.0, installed: 4.9.3]
  - mkdocs [required: >=1.1.0,<2, installed: 1.5.3]
    - click [required: >=7.0, installed: 8.1.7]
    - ghp-import [required: >=1.0, installed: 2.1.0]
      - python-dateutil [required: >=2.8.1, installed: 2.8.2]
        - six [required: >=1.5, installed: 1.16.0]
    - jinja2 [required: >=2.11.1, installed: 3.1.2]
      - MarkupSafe [required: >=2.0, installed: 2.1.3]
    - markdown [required: >=3.2.1, installed: 3.5]
    - markupsafe [required: >=2.0.1, installed: 2.1.3]
    - mergedeep [required: >=1.3.4, installed: 1.3.4]
    - packaging [required: >=20.5, installed: 23.2]
    - pathspec [required: >=0.11.1, installed: 0.11.2]
    - platformdirs [required: >=2.2.0, installed: 3.11.0]
    - pyyaml [required: >=5.1, installed: 6.0.1]
    - pyyaml-env-tag [required: >=0.1, installed: 0.1]
      - pyyaml [required: Any, installed: 6.0.1]
    - watchdog [required: >=2.0, installed: 3.0.0]
pre-commit==3.5.0
  - cfgv [required: >=2.0.0, installed: 3.4.0]
  - identify [required: >=1.0.0, installed: 2.5.30]
  - nodeenv [required: >=0.11.1, installed: 1.8.0]
    - setuptools [required: Any, installed: 68.2.2]
  - pyyaml [required: >=5.1, installed: 6.0.1]
  - virtualenv [required: >=20.10.0, installed: 20.24.6]
    - distlib [required: >=0.3.7,<1, installed: 0.3.7]
    - filelock [required: >=3.12.2,<4, installed: 3.12.4]
    - platformdirs [required: >=3.9.1,<4, installed: 3.11.0]
PyGithub==2.1.1
  - Deprecated [required: Any, installed: 1.2.14]
    - wrapt [required: >=1.10,<2, installed: 1.15.0]
  - pyjwt [required: >=2.4.0, installed: 2.8.0]
  - pynacl [required: >=1.4.0, installed: 1.5.0]
    - cffi [required: >=1.4.1, installed: 1.16.0]
      - pycparser [required: Any, installed: 2.21]
  - python-dateutil [required: Any, installed: 2.8.2]
    - six [required: >=1.5, installed: 1.16.0]
  - requests [required: >=2.14.0, installed: 2.31.0]
    - certifi [required: >=2017.4.17, installed: 2023.7.22]
    - charset-normalizer [required: >=2,<4, installed: 3.3.0]
    - idna [required: >=2.5,<4, installed: 3.4]
    - urllib3 [required: >=1.21.1,<3, installed: 2.0.7]
  - typing-extensions [required: >=4.0.0, installed: 4.8.0]
  - urllib3 [required: >=1.26.0, installed: 2.0.7]
python-dotenv==1.0.0
python-frontmatter==1.0.0
  - PyYAML [required: Any, installed: 6.0.1]

@suricactus
Copy link
Collaborator Author

Yes, this is correct. I just used pip freeze. There are pros and cons in both cases, I do prefer having all dependencies pinned as it guarantees better reproducability, but it is more a matter of taste.

@why-not-try-calmer
Copy link
Contributor

why-not-try-calmer commented Oct 24, 2023

Yes, this is correct. I just used pip freeze. There are pros and cons in both cases, I do prefer having all dependencies pinned as it guarantees better reproducability, but it is more a matter of taste.

But if mkdocs switches trans. dep 1 from 1A to 1B and we pin 1A, are we not guaranteeing the reproducibility of a broken build? I don't really see how this would improve on the situation where only non-trans. deps. are pinned. What problem does that solve?

@suricactus
Copy link
Collaborator Author

But if mkdocs switches trans. dep 1 from 1A to 1B and we pin 1A, are we not guaranteeing the reproducibility of a broken build?

If mkdocs changes dep 1A to 1B, we need to change the version of mkdocs too. If they require dep to be 1B, then our requirements.txt will fail to install, as we said ==1A, but mkdocs requires ==1B and we should react.

Anyways, both has pros and cons, I put only the direct dependencies in requirements.txt and we can leave the discussion about pros and cons for better times.

Copy link
Contributor

@why-not-try-calmer why-not-try-calmer left a comment

Choose a reason for hiding this comment

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

I much prefer without the transitive dependencies for this particular use case, thanks!

@suricactus
Copy link
Collaborator Author

Thanks for the review and green light! Let's hope it all build correctly!

@suricactus suricactus merged commit 2c8b7e6 into master Oct 24, 2023
1 check passed
@suricactus suricactus deleted the bump_deps branch October 24, 2023 13:35
@why-not-try-calmer
Copy link
Contributor

why-not-try-calmer commented Oct 24, 2023

Thanks for the review and green light! Let's hope it all build correctly!

I built locally the docs using your branch. Should someone/ I add a build step to the tests?

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.

3 participants