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

Feat/date time #21

Closed
wants to merge 14 commits into from
50 changes: 6 additions & 44 deletions .github/workflows/build_tests.yml
Original file line number Diff line number Diff line change
@@ -1,49 +1,11 @@
name: Run Build Tests
name: Unit Tests
on:
push:
branches:
- master
pull_request:
branches:
- dev
workflow_dispatch:

jobs:
build_tests:
strategy:
max-parallel: 2
matrix:
python-version: [ 3.7, 3.8, 3.9, "3.10" ]
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v3
- name: Setup Python
uses: actions/setup-python@v3
with:
python-version: ${{ matrix.python-version }}
- name: Install Build Tools
run: |
python -m pip install build wheel
- name: Install System Dependencies
run: |
sudo apt-get update
sudo apt install python3-dev swig libssl-dev
- name: Build Source Packages
run: |
python setup.py sdist
- name: Build Distribution Packages
run: |
python setup.py bdist_wheel
- name: Install repo
run: |
pip install .
- uses: pypa/[email protected]
with:
# Ignore irrelevant Mercurial vulnerability
# Ignore `requests` and `urllib3` vulnerabilities as they are not used in this package
# Ignore `setuptools` and `pip` vulnerabilities I don't think they apply here
ignore-vulns: |
PYSEC-2023-228
GHSA-9wx4-h78v-vm56
GHSA-34jh-p97f-mpxf
PYSEC-2022-43012
py_build_tests:
uses: neongeckocom/.github/.github/workflows/python_build_tests.yml@master
with:
python_version: "3.8"

Comment on lines +7 to +10
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Reconsider Python version coverage.

A previous review suggested including Python 3.11 in the test matrix. While the current implementation has moved away from a matrix approach, it's worth reconsidering the Python versions you're testing against.

If your project supports multiple Python versions, consider updating the workflow to test against them:

with:
  python_version: '["3.8", "3.9", "3.10", "3.11"]'

This ensures compatibility across supported Python versions, including the latest stable release (3.11).

36 changes: 4 additions & 32 deletions .github/workflows/license_tests.yml
Original file line number Diff line number Diff line change
@@ -1,39 +1,11 @@
name: Run License Tests
on:
push:
branches:
- master
workflow_dispatch:
pull_request:
branches:
- dev
workflow_dispatch:

- master
jobs:
license_tests:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v3
- name: Setup Python
uses: actions/setup-python@v3
with:
python-version: 3.8
- name: Install Build Tools
run: |
python -m pip install build wheel
- name: Install System Dependencies
run: |
sudo apt-get update
sudo apt install python3-dev swig libssl-dev
- name: Install core repo
run: |
pip install .
- name: Check python
id: license_check_report
uses: pilosus/[email protected]
with:
fail: 'Copyleft,Other,Error'
fails-only: true
exclude-license: '^(Mozilla).*$'
- name: Print report
if: ${{ always() }}
run: echo "${{ steps.license_check_report.outputs.report }}"
uses: neongeckocom/.github/.github/workflows/license_tests.yml@master

2 changes: 1 addition & 1 deletion .github/workflows/publish_stable.yml
Original file line number Diff line number Diff line change
Expand Up @@ -69,4 +69,4 @@ jobs:
uses: ad-m/github-push-action@master
with:
github_token: ${{ secrets.GITHUB_TOKEN }}
branch: dev
branch: dev
32 changes: 17 additions & 15 deletions .github/workflows/release_workflow.yml
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ on:

jobs:
publish_alpha:
if: github.event.pull_request.merged == true
uses: TigreGotico/gh-automations/.github/workflows/publish-alpha.yml@master
secrets: inherit
with:
Expand All @@ -17,6 +18,22 @@ jobs:
publish_prerelease: true
changelog_max_issues: 100

notify:
if: github.event.pull_request.merged == true
needs: publish_alpha
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v2
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Update actions/checkout action version.

The static analysis tool has flagged that the version of the actions/checkout action (v2) is outdated. It's recommended to update to the latest version to ensure compatibility and access to the latest features and bug fixes.

Please update the action version as follows:

- - uses: actions/checkout@v2
+ - uses: actions/checkout@v3

This change will ensure you're using the most up-to-date version of the checkout action.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
- uses: actions/checkout@v2
- uses: actions/checkout@v3
🧰 Tools
🪛 actionlint

26-26: the runner of "actions/checkout@v2" action is too old to run on GitHub Actions. update the action's version to fix this issue

(action)

- name: Send message to Matrix bots channel
id: matrix-chat-message
uses: fadenb/[email protected]
with:
homeserver: 'matrix.org'
token: ${{ secrets.MATRIX_TOKEN }}
channel: '!WjxEKjjINpyBRPFgxl:krbel.duckdns.org'
message: |
new ${{ github.event.repository.name }} PR merged! https://github.com/${{ github.repository }}/pull/${{ github.event.number }}

publish_pypi:
needs: publish_alpha
if: success() # Ensure this job only runs if the previous job succeeds
Expand All @@ -36,20 +53,6 @@ jobs:
- name: version
run: echo "::set-output name=version::$(python setup.py --version)"
id: version
- name: Create Release
id: create_release
uses: actions/create-release@v1
env:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} # This token is provided by Actions, you do not need to create your own token
with:
tag_name: V${{ steps.version.outputs.version }}
release_name: Release ${{ steps.version.outputs.version }}
body: |
Changes in this Release
${{ steps.changelog.outputs.changelog }}
draft: false
prerelease: true
commitish: dev
- name: Build Distribution Packages
run: |
python setup.py sdist bdist_wheel
Expand Down Expand Up @@ -102,4 +105,3 @@ jobs:
-H "Authorization: token $GITHUB_TOKEN" \
-d "{\"title\":\"$PR_TITLE\",\"body\":\"$PR_BODY\",\"head\":\"$HEAD_BRANCH\",\"base\":\"$BASE_BRANCH\"}" \
https://api.github.com/repos/${{ github.repository }}/pulls

69 changes: 66 additions & 3 deletions ovos_PHAL_plugin_mk1/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,9 @@ def __init__(self, bus=None, config=None):
self.bus.on("mycroft.audio.service.play", self.on_music)
self.bus.on("mycroft.audio.service.stop", self.on_display_reset)

self.bus.on("ovos.mk1.display_date", self.on_display_date)
self.bus.on("ovos.mk1.display_time", self.on_display_time)

self.bus.emit(Message("system.factory.reset.register",
{"skill_id": "ovos-phal-plugin-mk1"}))

Expand Down Expand Up @@ -512,14 +515,16 @@ def on_display(self, message=None):
on the faceplate at once.
"""
code = ""
x_offset = ""
y_offset = ""
x_offset = 0
y_offset = 0
clear_previous = ""
LOG.debug(message)
LOG.debug(message.data)
if message and message.data:
code = message.data.get("img_code", code)
x_offset = int(message.data.get("xOffset", x_offset))
y_offset = int(message.data.get("yOffset", y_offset))
clear_previous = message.data.get("clearPrev", clear_previous)
clear_previous = message.data.get("clear_previous", clear_previous)

clear_previous = int(str(clear_previous) == "True")
clear_previous = "cP=" + str(clear_previous) + ","
Expand Down Expand Up @@ -589,3 +594,61 @@ def on_weather_display(self, message=None):
icon = "x=2," + icon
msg = "weather.display=" + str(temp) + "," + str(icon)
self.writer.write(msg)

# date/time
def on_display_date(self, message=None):
self._deactivate_mouth_events()
self.on_text(message)
sleep(10)
self.on_display_reset()
self._activate_mouth_events()

def on_display_time(self, message=None):
code_dict = {
':': 'CIICAA',
'0': 'EIMHEEMHAA',
'1': 'EIIEMHAEAA',
'2': 'EIEHEFMFAA',
'3': 'EIEFEFMHAA',
'4': 'EIMBABMHAA',
'5': 'EIMFEFEHAA',
'6': 'EIMHEFEHAA',
'7': 'EIEAEAMHAA',
'8': 'EIMHEFMHAA',
'9': 'EIMBEBMHAA',
}

self._deactivate_mouth_events()
display_time = message.data.get("text")
# clear screen (draw two blank sections, numbers cover rest)
if len(display_time) == 4:
# for 4-character times, 9x8 blank
self.on_display(Message("", data={"img_code": "JIAAAAAAAAAAAAAAAAAA", "clear_previous": False}))
# self.enclosure.mouth_display(img_code="JIAAAAAAAAAAAAAAAAAA",
# refresh=False)
self.on_display(Message("", data={"img_code": "JIAAAAAAAAAAAAAAAAAA", "xOffset": 22, "clear_previous": False}))
# self.enclosure.mouth_display(img_code="JIAAAAAAAAAAAAAAAAAA",
# x=22, refresh=False)
else:
# for 5-character times, 7x8 blank
self.on_display(Message("", data={"img_code": "HIAAAAAAAAAAAAAA", "clear_previous": False}))
# self.enclosure.mouth_display(img_code="HIAAAAAAAAAAAAAA",
# refresh=False)
self.on_display(Message("", data={"img_code": "HIAAAAAAAAAAAAAA", "xOffset": 24, "clear_previous": False}))
# self.enclosure.mouth_display(img_code="HIAAAAAAAAAAAAAA",
# x=24, refresh=False)

# draw the time, centered on display
xoffset = (32 - (4 * (len(display_time)) - 2)) / 2
for c in display_time:
if c in code_dict:
self.on_display(Message("", data={"img_code": code_dict[c], "xOffset": xoffset, "clear_previous": False}))
if c == ":":
xoffset += 2 # colon is 1 pixels + a space
else:
xoffset += 4 # digits are 3 pixels + a space

self.on_display(Message("", data={"img_code": "CIAAAA", "xOffset": 29, "clear_previous": False}))
sleep(5)
self.on_display_reset()
self._activate_mouth_events()
Comment on lines +606 to +654
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Ensure xoffset is an integer to prevent potential type errors

In the on_display_time method, xoffset is calculated using division, which results in a float value. Since the xOffset parameter should be an integer, this could cause issues when positioning the display elements. Please use integer division to ensure xoffset is an integer.

Apply this diff to fix the issue:

-            xoffset = (32 - (4 * (len(display_time)) - 2)) / 2
+            xoffset = (32 - (4 * (len(display_time)) - 2)) // 2
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
def on_display_time(self, message=None):
code_dict = {
':': 'CIICAA',
'0': 'EIMHEEMHAA',
'1': 'EIIEMHAEAA',
'2': 'EIEHEFMFAA',
'3': 'EIEFEFMHAA',
'4': 'EIMBABMHAA',
'5': 'EIMFEFEHAA',
'6': 'EIMHEFEHAA',
'7': 'EIEAEAMHAA',
'8': 'EIMHEFMHAA',
'9': 'EIMBEBMHAA',
}
self._deactivate_mouth_events()
display_time = message.data.get("text")
# clear screen (draw two blank sections, numbers cover rest)
if len(display_time) == 4:
# for 4-character times, 9x8 blank
self.on_display(Message("", data={"img_code": "JIAAAAAAAAAAAAAAAAAA", "clear_previous": False}))
# self.enclosure.mouth_display(img_code="JIAAAAAAAAAAAAAAAAAA",
# refresh=False)
self.on_display(Message("", data={"img_code": "JIAAAAAAAAAAAAAAAAAA", "xOffset": 22, "clear_previous": False}))
# self.enclosure.mouth_display(img_code="JIAAAAAAAAAAAAAAAAAA",
# x=22, refresh=False)
else:
# for 5-character times, 7x8 blank
self.on_display(Message("", data={"img_code": "HIAAAAAAAAAAAAAA", "clear_previous": False}))
# self.enclosure.mouth_display(img_code="HIAAAAAAAAAAAAAA",
# refresh=False)
self.on_display(Message("", data={"img_code": "HIAAAAAAAAAAAAAA", "xOffset": 24, "clear_previous": False}))
# self.enclosure.mouth_display(img_code="HIAAAAAAAAAAAAAA",
# x=24, refresh=False)
# draw the time, centered on display
xoffset = (32 - (4 * (len(display_time)) - 2)) / 2
for c in display_time:
if c in code_dict:
self.on_display(Message("", data={"img_code": code_dict[c], "xOffset": xoffset, "clear_previous": False}))
if c == ":":
xoffset += 2 # colon is 1 pixels + a space
else:
xoffset += 4 # digits are 3 pixels + a space
self.on_display(Message("", data={"img_code": "CIAAAA", "xOffset": 29, "clear_previous": False}))
sleep(5)
self.on_display_reset()
self._activate_mouth_events()
def on_display_time(self, message=None):
code_dict = {
':': 'CIICAA',
'0': 'EIMHEEMHAA',
'1': 'EIIEMHAEAA',
'2': 'EIEHEFMFAA',
'3': 'EIEFEFMHAA',
'4': 'EIMBABMHAA',
'5': 'EIMFEFEHAA',
'6': 'EIMHEFEHAA',
'7': 'EIEAEAMHAA',
'8': 'EIMHEFMHAA',
'9': 'EIMBEBMHAA',
}
self._deactivate_mouth_events()
display_time = message.data.get("text")
# clear screen (draw two blank sections, numbers cover rest)
if len(display_time) == 4:
# for 4-character times, 9x8 blank
self.on_display(Message("", data={"img_code": "JIAAAAAAAAAAAAAAAAAA", "clear_previous": False}))
# self.enclosure.mouth_display(img_code="JIAAAAAAAAAAAAAAAAAA",
# refresh=False)
self.on_display(Message("", data={"img_code": "JIAAAAAAAAAAAAAAAAAA", "xOffset": 22, "clear_previous": False}))
# self.enclosure.mouth_display(img_code="JIAAAAAAAAAAAAAAAAAA",
# x=22, refresh=False)
else:
# for 5-character times, 7x8 blank
self.on_display(Message("", data={"img_code": "HIAAAAAAAAAAAAAA", "clear_previous": False}))
# self.enclosure.mouth_display(img_code="HIAAAAAAAAAAAAAA",
# refresh=False)
self.on_display(Message("", data={"img_code": "HIAAAAAAAAAAAAAA", "xOffset": 24, "clear_previous": False}))
# self.enclosure.mouth_display(img_code="HIAAAAAAAAAAAAAA",
# x=24, refresh=False)
# draw the time, centered on display
xoffset = (32 - (4 * (len(display_time)) - 2)) // 2
for c in display_time:
if c in code_dict:
self.on_display(Message("", data={"img_code": code_dict[c], "xOffset": xoffset, "clear_previous": False}))
if c == ":":
xoffset += 2 # colon is 1 pixels + a space
else:
xoffset += 4 # digits are 3 pixels + a space
self.on_display(Message("", data={"img_code": "CIAAAA", "xOffset": 29, "clear_previous": False}))
sleep(5)
self.on_display_reset()
self._activate_mouth_events()

4 changes: 4 additions & 0 deletions ovos_PHAL_plugin_mk1/arduino.py
Original file line number Diff line number Diff line change
Expand Up @@ -142,15 +142,19 @@ def __init__(self, serial, bus, size=16):
self.start()

def flush(self):
LOG.debug(f"alive in flush {self.is_alive}")
while self.alive:
try:
cmd = self.commands.get() + '\n'
LOG.debug(f"in arduino.flush {cmd}")
self.serial.write(cmd.encode())
self.commands.task_done()
except Exception as e:
LOG.error("Writing error: {0}".format(e))
LOG.debug("not alive anymore")

def write(self, command):
LOG.debug(f"command in writer write {command}")
self.commands.put(str(command))

def stop(self):
Expand Down
26 changes: 26 additions & 0 deletions scripts/remove_alpha.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
"""
on merge to master -> declare stable (remove alpha)
"""
import argparse
import fileinput
from os.path import abspath


def update_alpha(version_file):
alpha_var_name = "VERSION_ALPHA"

for line in fileinput.input(version_file, inplace=True):
if line.startswith(alpha_var_name):
print(f"{alpha_var_name} = 0")
else:
print(line.rstrip('\n'))
Comment on lines +9 to +16
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Improve error handling and add feedback in update_alpha function

The function correctly modifies the file, but it lacks error handling and doesn't provide feedback. Consider the following improvements:

  1. Add error handling for file operations.
  2. Provide feedback on whether the file was modified.
  3. Handle the case where VERSION_ALPHA is not found in the file.

Here's a suggested improvement:

def update_alpha(version_file):
    alpha_var_name = "VERSION_ALPHA"
    modified = False

    try:
        with fileinput.input(version_file, inplace=True) as file:
            for line in file:
                if line.startswith(alpha_var_name):
                    print(f"{alpha_var_name} = 0")
                    modified = True
                else:
                    print(line.rstrip('\n'))

        if not modified:
            print(f"Warning: {alpha_var_name} not found in {version_file}")
        else:
            print(f"Successfully updated {version_file}")

    except IOError as e:
        print(f"Error updating {version_file}: {e}")
        raise

    return modified

This version adds error handling, provides feedback, and returns a boolean indicating whether the file was modified.



if __name__ == "__main__":
parser = argparse.ArgumentParser(
description='Update the version based on the specified part (major, minor, build, alpha)')
parser.add_argument('--version-file', help='Path to the version.py file', required=True)

args = parser.parse_args()

update_alpha(abspath(args.version_file))
Comment on lines +19 to +26
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Enhance main block with error handling and user feedback

The main block correctly sets up argparse and calls the update_alpha function, but it lacks error handling and user feedback. Consider the following improvements:

  1. Add error handling for the update_alpha function call.
  2. Provide clear output to the user about the success or failure of the operation.
  3. Use a more descriptive help message for the --version-file argument.

Here's a suggested improvement:

if __name__ == "__main__":
    parser = argparse.ArgumentParser(
        description='Remove alpha designation from the version file by setting VERSION_ALPHA to 0')
    parser.add_argument('--version-file', 
                        help='Path to the version.py file to be updated', 
                        required=True)

    args = parser.parse_args()

    try:
        version_file = abspath(args.version_file)
        if update_alpha(version_file):
            print(f"Successfully removed alpha designation in {version_file}")
        else:
            print(f"No changes were made to {version_file}")
    except Exception as e:
        print(f"Error: Failed to update {args.version_file}")
        print(f"Reason: {str(e)}")
        exit(1)

This version adds error handling, provides clear user feedback, and uses a more descriptive help message for the --version-file argument.

65 changes: 65 additions & 0 deletions scripts/update_version.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
"""
on merge to dev:
- depending on labels (conventional commits) script is called with "major", "minor", "build", "alpha"
- on merge to dev, update version.py string to enforce semver
"""

import argparse
from os.path import abspath

def read_version(version_file):
VERSION_MAJOR = 0
VERSION_MINOR = 0
VERSION_BUILD = 0
VERSION_ALPHA = 0

with open(version_file, 'r') as file:
content = file.read()
for l in content.split("\n"):
l = l.strip()
if l.startswith("VERSION_MAJOR"):
VERSION_MAJOR = int(l.split("=")[-1])
elif l.startswith("VERSION_MINOR"):
VERSION_MINOR = int(l.split("=")[-1])
elif l.startswith("VERSION_BUILD"):
VERSION_BUILD = int(l.split("=")[-1])
elif l.startswith("VERSION_ALPHA"):
VERSION_ALPHA = int(l.split("=")[-1])
return VERSION_MAJOR, VERSION_MINOR, VERSION_BUILD, VERSION_ALPHA
Comment on lines +10 to +28
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Improve readability and robustness of read_version function.

The function logic is correct, but there are a few areas for improvement:

  1. Replace the single-letter variable l with a more descriptive name like line.
  2. Consider using a more robust parsing method, such as regular expressions, to extract version numbers.
  3. Add error handling for unexpected file formats or missing version components.

Here's a suggested refactoring:

import re

def read_version(version_file):
    version_components = {
        "VERSION_MAJOR": 0,
        "VERSION_MINOR": 0,
        "VERSION_BUILD": 0,
        "VERSION_ALPHA": 0
    }
    
    pattern = re.compile(r'^(VERSION_[A-Z]+)\s*=\s*(\d+)')
    
    with open(version_file, 'r') as file:
        for line in file:
            match = pattern.match(line.strip())
            if match:
                key, value = match.groups()
                if key in version_components:
                    version_components[key] = int(value)
    
    return tuple(version_components.values())

This refactoring improves readability, uses a more robust parsing method, and maintains the original function signature.

🧰 Tools
🪛 Ruff

18-18: Ambiguous variable name: l

(E741)


19-19: Ambiguous variable name: l

(E741)



def update_version(part, version_file):
VERSION_MAJOR, VERSION_MINOR, VERSION_BUILD, VERSION_ALPHA = read_version(version_file)

if part == 'major':
VERSION_MAJOR += 1
VERSION_MINOR = 0
VERSION_BUILD = 0
VERSION_ALPHA = 1
elif part == 'minor':
VERSION_MINOR += 1
VERSION_BUILD = 0
VERSION_ALPHA = 1
elif part == 'build':
VERSION_BUILD += 1
VERSION_ALPHA = 1
elif part == 'alpha':
VERSION_ALPHA += 1

with open(version_file, 'w') as file:
file.write(f"""# START_VERSION_BLOCK
VERSION_MAJOR = {VERSION_MAJOR}
VERSION_MINOR = {VERSION_MINOR}
VERSION_BUILD = {VERSION_BUILD}
VERSION_ALPHA = {VERSION_ALPHA}
# END_VERSION_BLOCK""")
Comment on lines +31 to +55
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Enhance robustness and maintainability of update_version function.

The function correctly implements semantic versioning logic, but there are opportunities for improvement:

  1. Add error handling for file operations.
  2. Consider preserving other content in the version file, if any.
  3. Use constants for version parts to improve maintainability.

Here's a suggested refactoring:

from enum import Enum
import re

class VersionPart(Enum):
    MAJOR = 'major'
    MINOR = 'minor'
    BUILD = 'build'
    ALPHA = 'alpha'

def update_version(part: VersionPart, version_file: str):
    try:
        VERSION_MAJOR, VERSION_MINOR, VERSION_BUILD, VERSION_ALPHA = read_version(version_file)

        if part == VersionPart.MAJOR:
            VERSION_MAJOR += 1
            VERSION_MINOR = VERSION_BUILD = 0
            VERSION_ALPHA = 1
        elif part == VersionPart.MINOR:
            VERSION_MINOR += 1
            VERSION_BUILD = 0
            VERSION_ALPHA = 1
        elif part == VersionPart.BUILD:
            VERSION_BUILD += 1
            VERSION_ALPHA = 1
        elif part == VersionPart.ALPHA:
            VERSION_ALPHA += 1

        version_block = f"""# START_VERSION_BLOCK
VERSION_MAJOR = {VERSION_MAJOR}
VERSION_MINOR = {VERSION_MINOR}
VERSION_BUILD = {VERSION_BUILD}
VERSION_ALPHA = {VERSION_ALPHA}
# END_VERSION_BLOCK"""

        with open(version_file, 'r+') as file:
            content = file.read()
            file.seek(0)
            file.write(re.sub(r'# START_VERSION_BLOCK.*# END_VERSION_BLOCK', version_block, content, flags=re.DOTALL))
            file.truncate()

    except IOError as e:
        print(f"Error updating version file: {e}")
        raise

This refactoring adds error handling, preserves other file content, and uses an enum for version parts to improve maintainability.



if __name__ == "__main__":
parser = argparse.ArgumentParser(description='Update the version based on the specified part (major, minor, build, alpha)')
parser.add_argument('part', help='Part of the version to update (major, minor, build, alpha)')
parser.add_argument('--version-file', help='Path to the version.py file', required=True)

args = parser.parse_args()

update_version(args.part, abspath(args.version_file))
Comment on lines +58 to +65
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add error handling to the main execution block.

The argument parsing is implemented correctly, but there's room for improvement:

  1. Add error handling for the update_version function call.
  2. Validate the part argument against allowed values.

Here's a suggested improvement:

if __name__ == "__main__":
    parser = argparse.ArgumentParser(description='Update the version based on the specified part (major, minor, build, alpha)')
    parser.add_argument('part', choices=['major', 'minor', 'build', 'alpha'], help='Part of the version to update')
    parser.add_argument('--version-file', help='Path to the version.py file', required=True)

    args = parser.parse_args()

    try:
        update_version(VersionPart(args.part), abspath(args.version_file))
        print(f"Successfully updated {args.part} version in {args.version_file}")
    except Exception as e:
        print(f"Error updating version: {e}")
        exit(1)

This improvement adds error handling and validates the part argument against allowed values.

6 changes: 6 additions & 0 deletions version.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
# START_VERSION_BLOCK
VERSION_MAJOR = 0
VERSION_MINOR = 0
VERSION_BUILD = 0
VERSION_ALPHA = 1
# END_VERSION_BLOCK
Loading