Install llnl-scraper from PyPI
#229
Conversation
Now that support for newer versions of `cloc` is available from a version installable from PyPI it makes sense to adjust the requirement to reference a version string instead of installing from a commit in a GitHub repository.
Update the dependencies installed in the Python virtual environment by running `pipenv lock` in the `src/` directory.
mcdonnnj
added
improvement
There was a problem hiding this comment.
Pull Request Overview
This PR switches the llnl-scraper dependency from a direct GitHub tarball to a PyPI release, bumps the project version to 0.3.0-rc.7, and updates all README references accordingly.
- Bump version in
src/version.txtfrom 0.3.0-rc.6 to 0.3.0-rc.7 - Change
llnl-scrapersource insrc/Pipfileto use>=0.15.0from PyPI - Update all occurrences of the old version in
README.mdto 0.3.0-rc.7
Reviewed Changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/version.txt | Bumped project version to 0.3.0-rc.7 |
| src/Pipfile | Replaced GitHub tarball pin with PyPI llnl-scraper >=0.15.0 |
| README.md | Updated Docker commands and documentation to use rc.7 tag |
Comments suppressed due to low confidence (1)
src/Pipfile:13
- [nitpick] Rewrite this comment for clarity, for example: "# Minimum version to support newer versions of cloc and avoid GitHub tarball pinning."
# Minimum version with newer versions of cloc compatibility
| # equivalent. | ||
| llnl-scraper = {file = "https://api.github.com/repos/LLNL/scraper/tarball/536a72ce1ceb2e209281ff72a2ed59e735d45c33"} | ||
| # Minimum version with newer versions of cloc compatibility | ||
| llnl-scraper = ">=0.15.0" |
There was a problem hiding this comment.
Consider adding an upper bound to this dependency (e.g., ">=0.15.0,<0.16.0") to follow semver and prevent unintended breaking changes, and regenerate the Pipfile.lock to lock down the updated version.
| llnl-scraper = ">=0.15.0" | |
| llnl-scraper = ">=0.15.0,<0.16.0" |
There was a problem hiding this comment.
Due to the slow developmental cycle of the llnl-scraper project I am not particularly worried about an upper bound. @cisagov/vm-dev any preference?
There was a problem hiding this comment.
I don't have strong feelings either way here, though it probably doesn't hurt to have an upper bound.
There was a problem hiding this comment.
So we can either do a <1 or just pin to a hard minor version because the history of releases for this project has been 0.x.0 versions.
There was a problem hiding this comment.
My vote is to pin to a specific minor version.
There was a problem hiding this comment.
I couldn't care less. Do what thou wilt.
There was a problem hiding this comment.
A key reminder here is that this is just a general requirements specification. It is only used when locking the lockfile. The lockfile specifies what specific version of a package is to be installed.
| # equivalent. | ||
| llnl-scraper = {file = "https://api.github.com/repos/LLNL/scraper/tarball/536a72ce1ceb2e209281ff72a2ed59e735d45c33"} | ||
| # Minimum version with newer versions of cloc compatibility | ||
| llnl-scraper = ">=0.15.0" |
There was a problem hiding this comment.
I don't have strong feelings either way here, though it probably doesn't hurt to have an upper bound.
🗣 Description
This pull request updates the llnl-scraper dependency to a version constraint so that it is installed from PyPI.
💭 Motivation and context
Now that a release with support for newer versions of cloc is available on PyPI we should prefer installation of the package from there. This resolves #148.
🧪 Testing
Automated tests pass.
✅ Pre-approval checklist
bump_versionscript if this repository is versioned and the changes in this PR warrant a version bump.✅ Post-merge checklist