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

Removed Semantic MediaWiki as a listed requirement #17

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

Conversation

yaronkoren
Copy link
Contributor

No description provided.

@JeroenDeDauw
Copy link
Member

Thanks for the patch. As it is, this leaves the repo in an inconsistent state. SMW is listed as concrete requirement in https://github.com/SemanticMediaWiki/SemanticSignup/blob/master/composer.json#L28 and at least used to be listed like that in SF as well. That would need to be changed to a suggest instead. And the docs can not just drop the SMW requirement as long as we support SF < 3.

@mwjames
Copy link
Contributor

mwjames commented Feb 9, 2015

-1

@mwjames mwjames closed this Feb 9, 2015
@JeroenDeDauw
Copy link
Member

@mwjames please don't just close this PR without providing a reason. Sure, it needs more work before it can be merged, though that does not mean the change is inherently bad.

@JeroenDeDauw JeroenDeDauw reopened this Feb 9, 2015
@mwjames
Copy link
Contributor

mwjames commented Feb 9, 2015

#16

@mwjames
Copy link
Contributor

mwjames commented Feb 9, 2015

Changes for 1.1 hasn't been merged nor collected but at this stage this PR is factual incorrect.

It can be re-submitted at the time such statement is factually correct (which would mean either SF 2.8 is longer supported or no other SMW related functionality has been incorporated).

@yaronkoren
Copy link
Contributor Author

I thought about the fact that SF 2.8 requires SMW. But it only makes sense to list direct requirements, no? Otherwise I would think you would also need to list Validator, DataValues, etc.

@mwjames
Copy link
Contributor

mwjames commented Feb 9, 2015

@JeroenDeDauw If you think this PR serves a purpose in order for users to make a more informed decision or are mislead by what is currently stated then feel free to ignore my -1.

@JeroenDeDauw
Copy link
Member

@yaronkoren at present, if you install this extension, SMW will also be installed. If SMW is not listed here, then users can end up being surprised at suddenly having it installed. Hence the package definitions first need updating.

@mwjames I'm not closing your PRs whenever I see an issue in them. And at present, I'm also -1 to this PR, but not -2.

@yaronkoren
Copy link
Contributor Author

Oh, okay. Well, that's beyond my expertise. Still, at least the URL should be fixed...

@JeroenDeDauw
Copy link
Member

@yaronkoren What URL? Just add SMW to the suggest section, as I did with SF and SRF for SMW in https://github.com/SemanticMediaWiki/SemanticMediaWiki/pull/793/files (I cannot add a commit on top of your PR)

@yaronkoren
Copy link
Contributor Author

Oops - I thought this was the other patch, where I modified the extension URL (#22). Please ignore that comment. I'll look into the "suggest" thing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants