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

print out fact version + git hash on start #1181

Merged
merged 2 commits into from
Mar 4, 2024
Merged

Conversation

jstucke
Copy link
Collaborator

@jstucke jstucke commented Dec 15, 2023

  • log fact version, git commit hash and python version on startup of each component
    • should help debugging some of the bare logs we get in GitHub issues

@jstucke jstucke requested a review from dorpvom December 15, 2023 09:09
@jstucke jstucke self-assigned this Dec 15, 2023
@maringuu maringuu self-requested a review January 9, 2024 07:47
Copy link
Collaborator

@maringuu maringuu left a comment

Choose a reason for hiding this comment

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

Nice change! I have some comments through.

Depending on gitpython installed (which in turn requires git) makes FACT even less packagable.
This is not a problem per se, as FACT currently cannot be packaged anyway but I think we should have packaging in mind.
I'd prefer if we dropped the gitpython dependency and use just git.
In addition, FACT should not crash if git is not installed.

For reference, here is what sway (a window manager, written in c) does in their meson.build to embed the git revision.

@jstucke
Copy link
Collaborator Author

jstucke commented Jan 9, 2024

Depending on gitpython installed (which in turn requires git) makes FACT even less packagable. This is not a problem per se, as FACT currently cannot be packaged anyway but I think we should have packaging in mind. I'd prefer if we dropped the gitpython dependency and use just git. In addition, FACT should not crash if git is not installed.

Why should gitpython be a problem but not git (since it is a pip package)? Also I think we can assume that git is installed if FACT is installed (since it gets used a lot during the installation and is explicitly installed in the pre_install.sh).

@maringuu
Copy link
Collaborator

Why should gitpython be a problem but not git [...]?

It is not a problem per-se, but I do not like adding dependencies for stuff that can be done in one line without the dependency.

@jstucke
Copy link
Collaborator Author

jstucke commented Feb 28, 2024

I replaced gitpython with a call to git

@jstucke jstucke requested a review from maringuu February 28, 2024 09:30
@maringuu maringuu merged commit 602ffca into master Mar 4, 2024
10 of 11 checks passed
@maringuu maringuu deleted the log-version-and-hash branch March 4, 2024 11:10
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