-
Notifications
You must be signed in to change notification settings - Fork 0
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
Release stability? #8
Comments
saml_sp is at full release. I see in contrib. modules some adding dependencies in composer.json and some not. We are using the latest version of saml_sp. Do we really need to pin this in dependency? |
It's not a blocker here, that's definitely true. I can see that it would only become critical if we need to enforce requiring a newer version of drupal/saml_sp. So, I'm happy to step away from it until that time. |
Chiming in even though it's closed! I see these as two separate questions:
Ideally, we'd follow best practices, but, in my 1.5 minutes of searching, I didn't find a guideline about these two questions. I posted on Drupal Slack, to see if anyone knows if there are best practices. Meanwhile, I spot-checked a bunch of "most used" modules: admin_toolbar, ctools, diff, inline_entity_form, paragraphs, pathauto, token.
My take, unless I get info from the Drupal community:
diff --git a/composer.json b/composer.json
index 5a14db3..dda6454 100644
--- a/composer.json
+++ b/composer.json
@@ -12,6 +12,7 @@
}
],
"support": {},
- "minimum-stability": "dev",
- "require": {}
+ "require": {
+ "drupal/saml_sp": "*"
+ }
} |
I added a PR, just to make it easy in case y'all agree 😉 |
https://github.com/CU-CommunityApps/CD_cwd_saml_mapping/blob/52092cdfa4acbcc2a6470b333a275369ed403e99/composer.json#L15
Seems like for v1.0.0 this should be "stable"? Unless drupal/saml_sp is not stable?
And as I think about this question of "what version of drupal/saml_sp are we using", this would be a reason I'd like to see that module marked as a composer dependency -- it assures we know what version we are working with and can pin it if we need to. The module info.yml file doesn't specify that requirement.
The text was updated successfully, but these errors were encountered: