Skip to content

Allow to set the URL for the security advisories composer url via env variable#5180

Merged
weitzman merged 4 commits intodrush-ops:11.xfrom
Cyberschorsch:feature/security-json-env-override
Jul 12, 2022
Merged

Allow to set the URL for the security advisories composer url via env variable#5180
weitzman merged 4 commits intodrush-ops:11.xfrom
Cyberschorsch:feature/security-json-env-override

Conversation

@Cyberschorsch
Copy link
Contributor

@Cyberschorsch Cyberschorsch commented Jul 11, 2022

The URL which is used in the drush pm:security command is hard-coded and links to the drupal-security-advisories repo ( https://github.com/drupal-composer/drupal-security-advisories ). I really would like to have an easy option to use a different url if needed.

This PR sets the url for the source of the security check command to the environment variable DRUSH_SECURITY_ADVISORIES_URL if set, otherwise use the default url.

@weitzman
Copy link
Member

Seems reasonable.

  • Please add a comment about this env variable not being a supported API.
  • Pkease use the null coalesce php operator to save some characters,

@Cyberschorsch
Copy link
Contributor Author

@weitzman thank your for the fast feedback, really appreciate it. I made the changes you suggested.

@weitzman
Copy link
Member

I see that the tests are failing. Hopefully its a quick fix.

@Cyberschorsch
Copy link
Contributor Author

Since getenv() returns FALSE instead of NULL if the env variable is not set or empty we propably have to stick with the ternary operator.

Copy link
Member

@weitzman weitzman left a comment

Choose a reason for hiding this comment

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

yeah, i was thinking of ternary. here is a suggested change

Co-authored-by: Moshe Weitzman <weitzman@tejasa.com>
@weitzman weitzman merged commit f324ff5 into drush-ops:11.x Jul 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants