-
Notifications
You must be signed in to change notification settings - Fork 293
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
fix: Remove outdated Pipenv requirements flag #780
fix: Remove outdated Pipenv requirements flag #780
Conversation
Thanks for fixing this - I starting running into it today. I forked your branch and ran the Looks like the |
Ah! Good catch! I ran the prettify:updated command and pushed. Thanks @rwestergren! |
1e66f94
to
7cc4301
Compare
7cc4301
to
5ac2729
Compare
@rwestergren I ran the validate workflow in my account to fix the remaining issues on commitlint, and things are now passing. I appreciate the heads-up. |
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.
Thanks a lot @jfgordon2 and sorry for not looking into it sooner. I have one question before merging. Do you think we shouldn't keep support for the older pipenv
versions that still have the --keep-outdated
flag? This is potentially a breaking change, even if the behavior might not be desirable by most people. If we drop the flag altogether, it would be great to update the docs and release it as a part of a new major version of the plugin
@pgrzesik I appreciate you taking a look! My understanding is that To me, it seems like the In this implementation, I'm following the Pipenv project's expectations that there will be a Pipfile.lock existing alongside the Pipfile, though added a warning just in case we don't see the expected files, and pointing a user to the Pipenv docs. We will see two potential benefits:
The |
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.
Thanks for clarification and your work on this one. 🙇
* master: Release v6.0.1 (serverless#793) ci: Temporarily disable test run on integrate (serverless#800) ci: Temporarily minimize testing matrix (serverless#799) ci: Fix test skips (serverless#797) ci: Temp skip of cache-related tests (serverless#796) ci: Remove node12 from testing matrix (serverless#795) fix: Not crash when runtime is not `python` (serverless#773) fix: Remove outdated Pipenv requirements flag (serverless#780) fix: Fix integration test matrix configuration (serverless#755) fix: Add legacy `pipenv` backward compatability (serverless#742) Add support for specifying custom dependency groups in Poetry (serverless#746) docs: Add contributing and code of conduct # Conflicts: # test.js
Removes use of the now-removed flag
--keep-outdated
on pipenv lock. See pipenv 2023.7.9 release notes.Rather than re-generating or potentially updating a user's Pipenv.lock file, it'll go straight into trying to build the requirements file.
The pre-existing behavior was that if the Pipfile.lock file was missing, that the plugin will generate one. Since this is likely undesireable, added a warning log message pointing a user to the Pipenv documenation on how to address it.
Also updates github-actions test matrix to include the before/after of this pipenv behavior change.
Closes: #779