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

Major Refactor, Dockerfile Changes, Unit Test Workflow, and Other Improvements #56

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

yeslayla
Copy link
Contributor

This pull request aims to modernize the project by updating it for newer versions of python, improving test coverage, reworking the Dockerfile, and a significant refactor of the code base. There is a decent amount of changes here, so let me know if there are any questions or feedback. Thanks!

Changes Made

  1. Refactoring
    a. SemVer class now lives within its own files at semver.semver and is not directly died into user input. Instead now, input from the CLI is passed to the SemVer class where the core logic still remains.
    b. Upgrade to use pyproject.toml Eventually deprecate setup.cfg with automatic conversion to pyproject.toml pypa/setuptools#3214
    c. Move git specifics functionality to a Git class using a SCM interface. This allows us to expand its functionality as needed. (I've considered adding Perforce support since its a popular SCM used in the gaming space.)
    d. .bumpversion.toml support in spirit with the general move to toml going on for python build tools.
    e. Generally tried to move code into classes as much as possible in hopes of simplifying complexity. I think this is most visible with how both the main entrypoint and the semver_get_version have shared logic going non in SemVer
    f. Improved test coverage with a preference to unit tests over integration tests. Lots of mocking going on and a MockSCM class was created to simplify testing without calling or needing to mock git commands.
    g. Formatting changes to better meet the PEP-8 guidelines.
    h. Type hints and docstrings using the format used by Jetbrain's Pycharm.
  2. GitHub Workflow
    a. test job runs the unit tests via pytest on a given matrix of . It's currently setup to run on Linux, Windows, and MacOS for every supported Python major version (as of July 22nd)
    b. coverage job is setup to gather the code coverage and comment a report on pull requests using https://github.com/MishaKav/pytest-coverage-comment
  3. Dockerfile
    a. Now uses a staged builder to create the wheels that are then copied to the final container. This avoids the final container containing unused files such as the .gitignore file or Jenkinsfile.
    b. Moved from CentOS container to Debain based slim python container.

Personal Message

Long time no see!

I want to acknowledge its been a while and I hope you are all doing well and thriving! Because I worked on auto-semver a lot during my time at RightBrain, I’ve wanted to update it to showcase as a portfolio piece. During my absence, I’ve gone through a lot of growth both personally and professionally. For those who remember me and wish to catch up, you’re welcome to reach out on LinkedIn or other social media.

Thank you for your time and support!

Warm regards,

Layla Manley

…rovements

* Update setup.py

* Update \setup.py to use `setup.cfg`

* Rework utils

* Major semver update and refactor

* Implement unittests

* Rework get_version and various improvements

* Get config properly

* Unit Test Workflow, Dockerfile, and Other Improvements
@yeslayla yeslayla requested a review from a team as a code owner July 22, 2023 12:08
@yeslayla
Copy link
Contributor Author

Note: Coverage Summary job is failing because the workflow only is able to generate tokens with read permissions rather than the permissions specified in the YAML.

https://docs.github.com/en/repositories/managing-your-repositorys-settings-and-features/enabling-features-for-your-repository/managing-github-actions-settings-for-a-repository

Remnant of migration
@adam-lewis-oberhausen
Copy link

Hi Laylay!!

First let me apologize for nobody responding to your PR. I'm relatively new to RightBrain but I think the work you've done on this is fantastic. As you've noted, this is a big PR. Can we setup a time to sync up and review this over a Zoom call?

@yeslayla
Copy link
Contributor Author

Hi Laylay!!

First let me apologize for nobody responding to your PR. I'm relatively new to RightBrain but I think the work you've done on this is fantastic. As you've noted, this is a big PR. Can we setup a time to sync up and review this over a Zoom call?

Hey Adam 👋

Yeah, sure! Warning, I'm on European time these days! Feel free to shoot me an email or reach out on LinkedIn in regards to scheduling. (My email should be on my LinkedIn)

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.

None yet

2 participants