Skip to content
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

Add a warning when multiple package managers are found #1692

Merged
merged 1 commit into from
Nov 8, 2024

Conversation

edmorley
Copy link
Member

@edmorley edmorley commented Nov 8, 2024

Currently the buildpack will use whichever package manager it finds first, if the files of multiple package managers are found.

This occasionally results in support tickets where the user believes the build to not be installing dependencies correctly, when in fact they are adding dependencies to the wrong package manager file.

As such, we want to make the presence of multiple package manager files an error, but we're first adding a warning so it's not a surprise. (Plus with the recent Poetry support addition, there will be apps using the third party Poetry buildpack that have both a poetry.lock and the generated requirements.txt until they remove the third-party buildpack.)

Towards #1691.
GUS-W-17167927.

@edmorley edmorley self-assigned this Nov 8, 2024
Currently the buildpack will use whichever package manager it
finds first, if the files of multiple package managers are found.

This occasionally results in support tickets where the user believes
the build to not be installing dependencies correctly, when in fact
they are adding dependencies to the wrong package manager file.

As such, we want to make the presence of multiple package manager
files an error, but first we should add a warning so it's not a surprise.

(Plus with the recent Poetry support addition, there will be apps using
the third party Poetry buildpack that have both a `poetry.lock` and
the generated `requirements.txt` until they remove the third-party
buildpack.)

Towards #1691.
GUS-W-17167927.
@edmorley edmorley force-pushed the multiple-package-manager-warning branch from 0872bcd to ce7296c Compare November 8, 2024 16:54
@edmorley edmorley marked this pull request as ready for review November 8, 2024 17:06
@edmorley edmorley requested a review from a team as a code owner November 8, 2024 17:06
@edmorley edmorley enabled auto-merge (squash) November 8, 2024 17:12
Copy link

@colincasey colincasey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Learning from my mistake by making the detection of multiple package managers a warning before making it an error 😅

@edmorley edmorley merged commit 0cd8efc into main Nov 8, 2024
7 checks passed
@edmorley edmorley deleted the multiple-package-manager-warning branch November 8, 2024 17:58
@heroku-linguist heroku-linguist bot mentioned this pull request Nov 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants