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

Prefer drupal namespace in Composer. #1020

Merged
merged 3 commits into from
Sep 4, 2024

Conversation

rosiel
Copy link
Member

@rosiel rosiel commented May 16, 2024

GitHub Issue: Islandora/documentation#2270

Other Relevant Links

  • Discussed as a possible reason for having trouble testing a pull request in slack DMs.

What does this Pull Request do?

Changes Islandora module to use the drupal composer namespace.

What's new?

  • Rename the module to drupal/islandora. This alone should have the effect of making it inaccessible to people requireing islandora/islandora... therefore:

  • Added a replace directive that says this package replaces islandora/islandora. This should make this package available to anyone who is requireing islandora/islandora, so long as they have the Drupal module repository in their composer file (and surely they must)

  • Changed our requirement for islandora/jsonld to drupal/jsonld.

  • Does this change add any new dependencies? It changes how an existing dependency is required.

  • Does this change require any other modifications to be made to the repository
    (i.e. Regeneration activity, etc.)? No!

  • Could this change impact execution of existing code? No.

How should this be tested?

[Help needed! It's not clear to me how to test this except to make a custom version of the starter site and require this branch? ]

With this PR, you should be able to get an islandora spun up (based on a starter site altered to remove the islandora/islandora dependency) and it would only have drupal/islandora.

With this PR merged, existing sites should not have a problem loading islandora/islandora or drupal/islandora.
[should we eventually mark packagist deprecated and suggest drupal/islandora?]

A description of what steps someone could take to:

  • Reproduce the problem you are fixing (if applicable)
  • Test that the Pull Request does what is intended.
  • Please be as detailed as possible.
  • Good testing instructions help get your PR completed faster.

Documentation Status

  • Does this change existing behaviour that's currently documented?
  • Does this change require new pages or sections of documentation?
  • Who does this need to be documented for?
  • Associated documentation pull request(s): ___ or documentation issue ___

Additional Notes:

Any additional information that you think would be helpful when reviewing this
PR.

Interested parties

Tag (@ mention) interested parties or, if unsure, @Islandora/committers

@rosiel rosiel marked this pull request as draft May 22, 2024 17:46
@rosiel
Copy link
Member Author

rosiel commented May 22, 2024

Decision made to "abandon" the packagist repo first, wait three months so people have time to update their sites, and then merge this PR (which could have un-obvious repercussions to sites using Packagist).

Decision (mine) made to not use composer.json to tag this package as deprecated (except as a final commit on the islandora/islandora namespace?).

In 3 months (Aug 22, 2024) merge this PR.

composer.json Outdated Show resolved Hide resolved
Co-authored-by: Adam <607975+adam-vessey@users.noreply.github.com>
@rosiel
Copy link
Member Author

rosiel commented Aug 29, 2024

It's been 3 months since the namespace retirement announcement, so this is ready for review.

Copy link

@aOelschlager aOelschlager left a comment

Choose a reason for hiding this comment

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

Approved in tech meeting.

@aOelschlager aOelschlager merged commit b85f318 into Islandora:2.x Sep 4, 2024
24 checks passed
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.

3 participants