-
Notifications
You must be signed in to change notification settings - Fork 10
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
MAPEX-27: Implement CI/CD System #2
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mehaase
requested changes
Sep 19, 2023
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are a LOT of changes here, which is great, but I don't want to roll out so many big changes all at the
same time. Let's prioritize a few ideas to try on this project, and let's defer the rest to future projects.
- Release please sounds like a great addition to our workflow but I left some notes on the documentation.
- While I like the idea that we could do a monorepo in the future, I don't want to separate the cli and
mappings_explorer packages. Let's keep those in one python package. - I'm not convinced about switching from make to invoke. Let's keep make for now.
- VS Code settings / extensions is great idea
I left questions on a few files.
mehaase
approved these changes
Sep 25, 2023
mikecarenzo
added a commit
that referenced
this pull request
Apr 15, 2024
…add_m365_mappings SSM-15: Add M365 Mappings to Explorer
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Implements #27
What Changed
This pull requests introduces a new CI/CD system.
New Project Structure
The
src
directory now maintains two completely independent packages.cli
: Contains the Mappings CLI Package.explorer
: Contains the Mappings Explorer Package.Each package defines their own:
mappings_*
tests
pytests
.README.md
pyproject.toml
tasks.py
It's important that we strictly separate each package. Separating the packages allows us to track the changes made to each package individually. This allows us to maintain separate change logs, separate version numbers, and (most importantly) separate releases.
Users that
pip install
the Mappings CLI only need the CLI, not the Explorer. Users that read the CLI's release notes only care about changes made to the CLI. Users that upgrade the CLI from one version to the next expect the version number to change only when changes to the CLI have been made. The same holds true for users of the Mappings Explorer.Yes, for now. However, this won't be an obstacle. Refer to Limitations for more information.
New Task Runner
make
has been replaced withinvoke
.While make is a powerful task runner, it's not pre-installed on Windows. We could instruct Windows developers to install it, but doing so would add a platform-specific step to the development process without offering any real advantages. make's syntax is generally foreign to most Python developers. As a result, Makefiles could prove difficult to maintain long-term.
invoke is a popular Python task runner designed specifically to emulate tools like make, but instead of relying on makefiles, it defines a
tasks.py
file.To test a package, simply run:
$ invoke test
To lint a package:
To lint AND test a package:
$ invoke lint test
To list the available tasks:
$ invoke --list Available tasks: build Build the Python package. lint Run black, ruff, and mypy test Run Python tests.
To get help on a specific task:
It's recommended that you install invoke globally (
pip install invoke
). However, if you choose not to, you can also run invoke inside apoetry shell
once youpoetry install
either project. You can alsopoetry run invoke
.For more information about
tasks.py
files, refer to What is Invoke?Added
release-please
GitHub ActionGoogle's
release-please
automatically tracks commits made to the repository'smain
branch (in the Conventional Commit format). It uses the commit messages to compute each package's version number and generate each package's change log.release-please
updates each version number and change log in a parallel "Release Pull Request". When we're ready to tag a release, we simply merge the Release PR.For more information about Release PRs, refer to the documentation.
This PR also introduces the
add_tags
GitHub Action. It's a custom action that offers us more control over how and when GitHub Releases are published.For more information about
add_tags
, refer to the README.Improved Workflow Performance
Because there are two independent packages, there are two independent test workflows. Each workflow is designed to lint, run test cases, and assess security vulnerabilities. Separating the tests ensures that both tests run. If both the CLI and Explorer were tested in sequence (under the same workflow) a failure in the CLI tests would prevent the Explorer from being tested.
To improve test performance, both workflows now leverage GitHub's Cache action. The first time a test workflow runs, it installs Poetry and the package's dependencies. After the workflow completes, it caches the Poetry installation and the package's virtual environment. Then, on each consecutive run, the workflow restores the Poetry installation and virtual environment from the cache (instead of redownloading and reconfiguring the environment). This cuts down the execution time for both workflows.
Both workflows are configured to destroy their cached virtual environment if the package's
poetry.lock
file changes. In this case, it will reinstall and recache the virtual environment.Updated Editor Configurations
Visual Studio Code has broken the Python extension into several separate extensions. As a result:
"python.formatting.autopep8Path"
"python.formatting.provider"
have been deprecated.
The
settings.json
file has been updated to reflect these new extensions and their settings.An
extensions.json
file has also been added. This file declares all the project's recommended VSCode Extensions so that new developers can immediately configure their development environment the first time the project launches.If the user clicks Show Recommendations, they'll be shown the recommended plugins:
Limitations
Both
pyproject.toml
files share a similar set of dependencies, configurations, and build options. Unfortunately, Poetry has limited support for monorepos. Although, that could be changing [1] [2].In the future, I'd like to develop a small script that unifies the virtual environments into a singular environment during development. Until then, they'll need to be separate. Both projects serve largely different purposes so this really shouldn't be much of an issue. There's no real case where a developer would need to frequently switch back and forth between the two environments.