feature : adds global version#355
Conversation
There was a problem hiding this comment.
Pull Request Overview
Adds a global --version flag to the CLI that displays version information without requiring a subcommand, improving user experience by following standard CLI conventions.
- Implements centralized version management through a new
__version__.pymodule - Adds global
--versionflag with dynamic version discovery fallback strategy - Updates tests to use the new version system instead of hardcoded values
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| cli/version.py | New module providing centralized version discovery with fallback strategy |
| cli/cli.py | Adds global --version flag and callback using the new version system |
| cli/test_cli.py | Adds test for global version flag and imports new version function |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
cli/__version__.py
Outdated
| - Lastly, fallback to "0.0.0" if nothing else is found | ||
| """ | ||
| try: | ||
| return pkg_version("linux-commands-cli") |
There was a problem hiding this comment.
The package name 'linux-commands-cli' is hardcoded here but differs from the APP_NAME constant. Consider using a constant or deriving one from the other to maintain consistency.
cli/__version__.py
Outdated
| except PackageNotFoundError: | ||
| pass | ||
|
|
||
| setup_path = os.path.join(os.path.dirname(__file__), "setup.py") |
There was a problem hiding this comment.
The setup.py file is being looked for in the cli/ directory, but setup.py files are typically located in the project root directory. This path should likely be os.path.join(os.path.dirname(__file__), '..', 'setup.py').
| setup_path = os.path.join(os.path.dirname(__file__), "setup.py") | |
| setup_path = os.path.join(os.path.dirname(__file__), '..', 'setup.py') |
bobbyiliev
left a comment
There was a problem hiding this comment.
Thanks for the PR! Seems like the Lint CI job is failing, woudl you mind taking a look? Also consider some of the suggestions from the Copilot bot.
|
Sure ! |
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
fix : lint issue
38e25d4 to
3f3777c
Compare
|
Hi @bobbyiliev |
|
Tests were still failing and it seems like the version command has already been contributed. I'm still marking this as hacktoberfest accepted, as it was a good contribution! |
What type of PR is this? (check all applicable)
[ ] ♻️ Refactor
[ ] ✨ New Chapter
[ ] 🐛 Bug Fix/Typo
[ ] 👷 Optimization
[ ] 📝 Documentation Update
[x] 🚩 Other
Description
Add a global --version flag to the CLI so users can run python cli.py --version without a subcommand.
Introduce cli/version.py with get_version_string() to centralize version information.
Implement dynamic version discovery:
Prefer the installed package version (linux-commands-cli) via importlib.metadata.
Fallback to parsing cli/setup.py when running from source.
Fallback to 0.0.0 if no version is found.
Add tests for the global --version flag and update tests to assert against get_version_string() instead of a hardcoded string.
Related Tickets & Documents
N/A
Added to documentation?
[ ] 📜 readme
[x] 🙅 no documentation needed
[optional] What gif best describes this PR or how it makes you feel?
N/A
If you prefer a different type classification, we could also tag this as “👷 Optimization” since it improves UX and testability without changing chapters.