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 audit.abandoned warnings for abandoned packages #11639

Merged
merged 1 commit into from Sep 14, 2023

Conversation

Seldaek
Copy link
Member

@Seldaek Seldaek commented Sep 13, 2023

Fixes #11623

{
if ($abandoned === 'default' && $format !== self::FORMAT_SUMMARY) {
Copy link
Contributor

Choose a reason for hiding this comment

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

default is not one of the values supported in the method signature for static analysis.

Copy link
Contributor

@curry684 curry684 Sep 13, 2023

Choose a reason for hiding this comment

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

I would define an extra constant ABANDONED_DEFAULT = 'default' for now and switch it to ABANDONED_DEFAULT = self::ABANDONED_FAIL in 2.7. Also this section needs a todo to remove the warning in 2.7

Copy link
Member Author

Choose a reason for hiding this comment

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

The point is default as a value will be gone in 2.7 so i rather keep it low profile. IMO it's fine as is

@@ -37,7 +38,7 @@ class Config
'allow-plugins' => [],
'use-parent-dir' => 'prompt',
'preferred-install' => 'dist',
'audit' => ['ignore' => []],
'audit' => ['ignore' => [], 'abandoned' => 'default'], // TODO in 2.7 switch to ABANDONED_FAIL
Copy link
Contributor

Choose a reason for hiding this comment

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

Referencing Auditor::ABANDONED_DEFAULT here automatically future-proofs the code.

@curry684
Copy link
Contributor

Awesome quick work man 👍

{
if ($abandoned === 'default' && $format !== self::FORMAT_SUMMARY) {
$io->writeError('<warning>The new audit.abandoned setting (currently defaulting to "report" will default to "fail" in Composer 2.7, make sure to set it to "report" or "ignore" explicitly by then if you do not want this.</warning>');
Copy link
Member

Choose a reason for hiding this comment

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

Should this warning really always be output or only if an actual warning is output? Seems a bit much to force everyone to set this to a value to make the warning go away?

Copy link
Member Author

Choose a reason for hiding this comment

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

Well if you run audit in CI you can also just ignore this warning.. my thinking was more that if it's hidden then you won't notice it unless you have abandoned packages. Then people will come complaining when 2.7 hits and they have an abandoned package in six months..

Copy link
Contributor

Choose a reason for hiding this comment

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

Is this really an issue? I highly doubt audit is frequently used outside CI context, so it's not really a 'daily annoyance' or anything, and those that do consciously run audit will be the kind of user happy with the verbose explanation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Audit should (have an option to) fail on abandoned packages
4 participants