Skip to content

Add commit revision to beta branch version display (#7768) #7820

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

Open
wants to merge 5 commits into
base: dev
Choose a base branch
from

Conversation

abstewart
Copy link

@abstewart abstewart commented Jul 22, 2024

Fixes #7768 .

Description of the problem being solved:

Added logic to update_manifest.py to also store the latest commit hash (short version) in the manifest.xml file.

This is then read in in Launch.lua and used in Main.lua when the branch is "beta" to display the Rev/Hash on the lower right.

Steps taken to verify a working solution:

  • Force disabled rev mode, copied the latest beta manifest.xml and added a fake hash to it, launched pob and confirmed that the hash is displayed when branch is "beta" but not when "master"

Link to a build that showcases this PR: any build on the beta branch

Before screenshot:

before

After screenshot:

after

Additional Notes

  • I'm unfamiliar with how the release process / installing updates works. I believe I've made all the edits for the hash to get updated with the beta build, but it's possible I missed something.
  • The recorded hash might not be the absolute latest (since it might not capture the merge commit hash), but it should always update when there's new changes.

@Paliak Paliak added enhancement New feature, calculation, or mod technical Hidden from release notes labels Jul 22, 2024
@abstewart
Copy link
Author

The current hash in manifest.xml doesn't exist since the commit it points to (the top one) was amended with the addition of the hash, and the hash was changed as a result. Future hashes will exist in the history as long as the change to the manifest.xml is committed on top of the history and not amended to the same commit (changing the hash).

Copy link
Member

@ppoelzl ppoelzl left a comment

Choose a reason for hiding this comment

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

NB: When you install packages into a Python project, you should always use a virtual environment for that project, if you haven't done so already. PyCharm can create one for you (Settings > Project: Path of Building > Python Interpreter
Add Interpreter > Add Local Interpreter > Virtualenv Environment).

Comment on lines 10 to +11
import xml.etree.ElementTree as Et
import subprocess
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
import xml.etree.ElementTree as Et
import subprocess
import subprocess
import xml.etree.ElementTree as Et

Sort imports in alphabetical order. You can use isort (pip install isort) to automatically sort imports.

@@ -49,6 +50,8 @@ def _alphanumeric(key: str) -> list[int | str]:
for character in re.split(alphanumeric_pattern, key)
]

def _get_latest_commit_hash_short() -> str:
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
def _get_latest_commit_hash_short() -> str:
def _latest_commit_hash_short() -> str:

Usually, you leave out get/set in Python functions or method names.

@@ -49,6 +50,8 @@ def _alphanumeric(key: str) -> list[int | str]:
for character in re.split(alphanumeric_pattern, key)
]

def _get_latest_commit_hash_short() -> str:
return subprocess.check_output(['git', 'rev-parse', '--short', 'HEAD']).decode('ascii').strip()
Copy link
Member

@ppoelzl ppoelzl Jul 23, 2024

Choose a reason for hiding this comment

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

Suggested change
return subprocess.check_output(['git', 'rev-parse', '--short', 'HEAD']).decode('ascii').strip()
return subprocess.run(
["git", "rev-parse", "--short", "HEAD"],
check=True,
stdout=subprocess.PIPE,
encoding="ascii",
).stdout.strip()

All of the API surface of subprocess is soft deprecated except subprocess.run and subprocess.Popen.
You can use subprocess.run to achieve the same result. You can use black (pip install black) to automatically format the code like suggested. The black style prefers double quotes over single quotes.

@@ -78,6 +81,7 @@ def create_manifest(version: str | None = None, replace: bool = False) -> None:
logging.critical(f"Manifest configuration file not found in path '{base_path}'")
return

versions = {"number":new_version, "hash":_get_latest_commit_hash_short()}
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
versions = {"number":new_version, "hash":_get_latest_commit_hash_short()}
versions = {"number": new_version, "hash": _latest_commit_hash_short()}

Changes made by black.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature, calculation, or mod technical Hidden from release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

More Detailed Version String for Beta Branch
3 participants