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

Prefix global identifiers with module name #10

Open
drunken-monkey opened this issue May 24, 2017 · 1 comment
Open

Prefix global identifiers with module name #10

drunken-monkey opened this issue May 24, 2017 · 1 comment

Comments

@drunken-monkey
Copy link

I didn't want to include this in my PR because I'd like to discuss it first and hear your opinion before I go and code it.
In general, the "best practice" for any global identifiers (like function/annotation names, service/plugin IDs, etc.) is to prefix them with the module name. (Class names in D8 are of course already taken care of with "real" namespaces.) The question is whether there's a reason why you're not following that for the plugin and service IDs in this module?
Of course, it's rather unlikely that there will be another Solr module in D8 (hopefully, at least), so as long as the search_api_solr maintainers are aware of this module, the potential for clashes isn't that big. And not having the complete module prefix would in any case be a plus if we later merge this module into search_api_solr itself. (However, for that, having "search_api_solr_" prefixes would be better, I'd say.) Finally, I of course realize having a 27-character prefix looks pretty ugly, so I see why you'd want to avoid that.
Still, personally I always like to do these things "by the book" and use proper prefixing even if it seems a pretty pointless drag. In the end, though, it's your decision as the maintainer, of course. (If we'd want to merge, we'd just have to ask Markus Kalkbrenner how he sees this. I gues the transition won't be completely smooth/trivial anyways, so changing plugin and service IDs probably won't add much to the problem.)

@DavidACameron-USDA
Copy link
Owner

DavidACameron-USDA commented May 24, 2017

These are the first services that I've written for a module. I don't think that I even considered prefixing them, but I really should have known better. Thanks for catching it. It's not a problem to update the names. I can take care of it since it was my mistake, but feel free if you want to do it.

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

2 participants