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

Release stability? #8

Open
woodseowl opened this issue Aug 28, 2023 · 4 comments
Open

Release stability? #8

woodseowl opened this issue Aug 28, 2023 · 4 comments

Comments

@woodseowl
Copy link

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.

@judaw
Copy link
Collaborator

judaw commented Aug 28, 2023

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?

@woodseowl
Copy link
Author

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.

@alisonjo315
Copy link
Contributor

alisonjo315 commented Aug 28, 2023

Chiming in even though it's closed!

I see these as two separate questions:

  1. "minimum-stability" (set it or not, and what to set it at)
  2. including saml_sp as a package requirement (not just an enabled module requirement, so to speak)

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.

  • Thing 1: Most of the modules I checked skip this setting; a few do have the setting, and they all use "dev". Idk the purpose of this setting in the context of a module composer file, except when there's a specific need?
    • In my spot-checking, ctools and inline_entity_form are the only modules with this setting.
  • Thing 2: Paragraphs and Pathauto are the only modules I checked that have dependencies.
    • Pathauto requires ctools and token, and it uses "*" as the version constraint for both.
    • Paragraphs requires entity_reference_revisions, version constraint "~1.3".
  • (If it would be helpful, I can provide more package details.)

My take, unless I get info from the Drupal community:

  • Let's drop the minimum-stability setting.
  • Let's add drupal/saml_sp as a package requirement, with "*" as the version constraint.
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": "*"
+    }
 }

@alisonjo315
Copy link
Contributor

I added a PR, just to make it easy in case y'all agree 😉

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

No branches or pull requests

3 participants