Merged
Conversation
Previously, the buildpack didn't track which package manager was being used, but instead had a number of fragile file presence checks and env vars like `SKIP_PIP_INSTALL` scattered throughout (which won't work when the package manager isn't a binary choice). As such, package manager handling has been refactored to allow for more easily adding Poetry support later. This also continues the gradual migration of code from sourced `bin/steps/` scripts, to functions declared in `lib/`. As part of this, I've started namespacing the lib functions using the `module::fn_name()` pattern described in: https://google.github.io/styleguide/shellguide.html#function-names On the most part I've tried to keep this refactoring a no-op (aside from fixes for a few quirks/bugs found whilst working on this). Longer term we will be deprecating/sunsetting several features, such as the `setup.py` fallback, using Pipenv without a lockfile, and support for apps that have ended up with the files for multiple package managers. GUS-W-16811078.
colincasey
approved these changes
Sep 24, 2024
colincasey
left a comment
colincasey
left a comment
There was a problem hiding this comment.
Looks okay to me @edmorley. Just had a couple of minor comments for you to respond to.
Follow-up to #1638.
Merged
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Previously, the buildpack didn't track which package manager was being used, but instead had a number of fragile file presence checks and env vars like
SKIP_PIP_INSTALLscattered throughout (which won't work when the package manager isn't a binary choice).As such, package manager handling has been refactored to allow for more easily adding Poetry support later.
This also continues the gradual migration of code from sourced
bin/steps/scripts, to functions declared inlib/. As part of this, I've started namespacing the lib functions using themodule::fn_name()pattern described in:https://google.github.io/styleguide/shellguide.html#function-names
On the most part I've tried to keep this refactoring a no-op (aside from fixes for a few quirks/bugs found whilst working on this). Longer term we will be deprecating/sunsetting several features, such as the
setup.pyfallback, using Pipenv without a lockfile, and support for apps that have ended up with the files for multiple package managers.Prep for #796 / #932.
GUS-W-16811078.