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

Additional readthedocs fixes #592

Merged
merged 2 commits into from
Jan 31, 2025
Merged

Conversation

achaikou
Copy link
Contributor

No description provided.

Readthedocs doesn't compile the code and thus fails.
The simplest solution is to change version manually with every release.
I however don't think it is used anywhere in the documentation, so it is
likely it could be safely deleted instead.
Copy link
Contributor

@ajaust ajaust left a comment

Choose a reason for hiding this comment

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

Looks good. 👍

# static analysis flags the subproces module as a potential security
# problem, but this takes no user input and should be safe
from subprocess import check_output # nosec
version = str(check_output(['git', 'describe']).strip()) # nosec
Copy link
Contributor

Choose a reason for hiding this comment

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

Just a finding that supports removal of this code part:
Out of curiosity I checked the output of git describe on my master branch. It gives me v1.9.6-76-g62c10d0 which I find surprising since the latest tag is v1.9.12. I wonder when/why git describe broke for this repository.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I never could understand this versioning thing, so good to see it seems properly broken and thus can be removed!

@achaikou achaikou merged commit f541296 into equinor:master Jan 31, 2025
17 checks passed
@achaikou achaikou deleted the readthedocs_version branch February 6, 2025 09:12
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.

2 participants