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

Remove SMW requirement from composer.json #787

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

yaronkoren
Copy link
Contributor

Of course SMW is required for SRF to run, but the presence of this line makes it impossible for SMW and SRF to both be downloaded via a non-Composer option like Git, because then calling "composer install" for SRF, in order to get its dependencies, will lead to SMW getting re-downloaded. The line doesn't seem necessary, anyway.

Of course SMW is required for SRF to run, but the presence of this line makes it impossible for SMW and SRF to both be downloaded via a non-Composer option like Git, because then calling "composer install" for SRF, in order to get its dependencies, will lead to SMW getting re-downloaded. The line doesn't seem necessary, anyway.
@hexmode
Copy link
Member

hexmode commented Aug 15, 2023

If the reason for this is because of a problem with composer vs git, why not just use --prefer-source when you run composer? That would get the SMW dependency via git.

@yaronkoren
Copy link
Contributor Author

That sounds better, but I think better yet would be to not get SMW at all. Why get it?

@hexmode
Copy link
Member

hexmode commented Aug 15, 2023

That sounds better, but I think better yet would be to not get SMW at all. Why get it?

It is a dependency and composer is a dependency manager, so naturally, it gets it.

If this PR is accepted, other people will come along and say "Oh, the SMW dependency is missing. Let me add it." People will be continually pointed back here to say "We're not including it so that composer DTRT in this one case. Yes, we know that it is wrong in some other cases, but we're not going to fix it. You just always have to specify SMW and SRF."

@yaronkoren
Copy link
Contributor Author

Well, disregarding for now any difficulties with future maintenance, what would you say is the preferred behavior? I think there's a strong case that Composer "require" should only be used for libraries (including "extension libraries" like DataValues), and not for real MediaWiki extensions, because those are unnecessary and can lead to unexpected behavior.

@hexmode
Copy link
Member

hexmode commented Aug 16, 2023

... and not for real MediaWiki extensions, because those are unnecessary and can lead to unexpected behavior.

I'm not sure what you mean here by "unnecessary". SMW is a necessary dependency for SRF, isn't it?

@yaronkoren
Copy link
Contributor Author

I mean that it's not necessary because the user can always include a line for these extensions in their composer.json file in they want them to be downloaded during this operation. (And presumably, in the vast majority of cases, they already will.) A user who doesn't want an extension like SMW to be downloaded during the operation (because they've already downloaded it separately), on the other hand, will have no recourse, as far as I can tell.

@krabina
Copy link
Contributor

krabina commented Aug 16, 2023

Your thoughts, @gesinn-it-gea?

@krabina krabina mentioned this pull request Nov 22, 2023
@krabina krabina mentioned this pull request Dec 7, 2023
@gesinn-it-gea
Copy link
Member

... I follow @yaronkoren reasoning. In my experience, dependency managers are good for managing libraries, but don't necessarily make things easier for installing extensions. Personally, I prefer to be able to take care of the dependencies myself.

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.

4 participants