Create interacting-with-core

Created by: eileenmcnaughton

Per discussion in documentation channel I think we need to document what methods are considered supported / expected to be stable. The last 4 items in this list have not been previously documented as a supported way to interact with CiviCRM so may be subject to discussion

Created by: seancolsen

@eileenmcnaughton thanks for this! These are helpful things to know!

I made a few changes here and I'd like you input on my changes before merging. I explained a bit in the commit messages, but I have a few more things to say here...

1. I don't think this topic is large enough to warrant its own page. Thus, I moved the content into the main "Extensions" page. In looking at that page, I realize now that the whole page needs quite a bit of work. Nonetheless, I think that's be best place for this content, but I'm open to other thoughts as well. If we really do want a separate page for this content, then we'll need to add an entry to mkdocs.yml for it.

2. I cleaned up the markdown a bit. However, it's worth mentioning that this issue is currently preventing my cleaned up markdown from displaying as nicely as I'd like.

3. I did make one conceptual change because I don't think this sentence is entirely accurate:

   ...only extensions that use fully supported methods are eligible for publication in the extensions directory

   I explained a bit more in the commit msg b0994c33 and I'm open to differing opinions here.

Created by: eileenmcnaughton

Regarding the extension criteria - if I were reviewing or installing or writing an extension I would want to ensure that only supported methods are used but it's up to the extension group to set the standards. I do my own review of how extensions are written before installing them but I think we should move towards expanding that list to the point where that statement would be true - ie. there might be other legit functions.

short version - your change is fine.

I do want @totten confirmation that he agrees these things ARE supported interfaces before merge

Created by: totten

I consider everything in the Civi:: facade to be supported (and tried to make a big deal about it a while back in the blog). I expect a high degree of scrutiny for a change in the Civi:: interfaces, and I think it makes sense to call/use those.

The only caveats would be:

• Civi::cache() has an explicit disclaimer in the docblock.
• For something like Civi::service($id), the main function is supported, but the overall list of possible $ids is a bit open-ended. Some id's might be extremely reliable, and... less so. We'd have to talk about the specific $id.

Created by: eileenmcnaughton

Hmm - I feel like we need to get something (like a change log) into docs if we are clearly supporting those interfaces.

What about the api listeners I mention? Existing docs imply they may only be supported for core

Created by: totten

Agree we should document civi.api.* events. I'm not certain where we'd put it, though. (Maybe https://docs.civicrm.org/dev/en/latest/framework/api-architecture/ ? Or maybe just document events in the same chapter/styling as hooks?)

Tangential: To defend the existing docs a little... I've tried to file information under topical sections. For example: Civi::settings() under "Settings" and token events under "Tokens" and FlexMailer events under "FlexMailer". That produces more of a topical/textbook-style than a changelog/newsfeed-style.

Regarding changelog/newsfeed-style formulations -- do you think we have enough media in the form of (a) the civicrm-dev-docs PR queue and (b) the curated release-notes for core? Or something else is needed?

Created by: eileenmcnaughton

Re change log - we have this https://docs.civicrm.org/dev/en/latest/api/changes/ for the API - I guess this discussion is about expanding the definition of the api and I could see a case for adding to that (although for the change in definition I would not try to retrospectively create a timeline but rather put in that the definition of the supported interfaces are formally changed to include...

Created by: eileenmcnaughton

regarding existing docs with civi interfaces - if we have a list of 'supported interfaces' it can just be bullets linking to other pages (I found some) and any variance overtime is supported by the change log. Part of 'supported' is probably the implication that there IS a change log

Created by: seancolsen

@eileenmcnaughton and @totten just a ping here to see what else is necessary to move this PR along so that it doesn't go stale... What else dose it need?

Created by: eileenmcnaughton

@totten I think we should merge this as is & add to it as we agree additional things are specifically supported. I don't think 'everything in Civi facade' works as there are a lot of internal methods - I think we want to be clearer than that

Created by: totten

(a) As long as this is just a list of links to other parts of the docs, I'm OK w/it at a high-level.

I don't think 'everything in Civi facade' works as there are a lot of internal methods

(b) I don't really follow. Might be better to go through the list and compare thoughts. When looking at Civi.php, these buckets stand out to me:

• Stable/unlikely to change:
  • Upstream/Symfony/PHP-FIG: ::container(), ::dispatcher(), ::log(), ::service()
  • civicrm-core: ::paths(), ::resources(), ::settings()
• Internal/meta: ::reset()
• Oddball: ::cache() -- Explicitly flagged as "EXPERIMENTAL". I'd suggest checking-up on PHP-FIG to see what's happened in the last year or two -- maybe they've had some resolution on how to standardize caching interfaces.
• Oddball: ::lockManager() -- I'd say there's a presumption of its continuation. I sorta wish there were a third-party/upstream standard instead, but I'm not aware of one, so keeping it seems like the smarter move.

Created by: totten

The combination of statements feels a little confusing: "These are supported to greater or lesser degrees." followed by "Fully supported methods are:".

Proposals:

• Easy: Just kill the second sentence.
• Difficult: Kill the second sentence. Additionally, update each bullet-point with a comment or rating about support/maturity.

Created by: eileenmcnaughton

I think the below are fully supported - I feel like there are lot of things in the report interface that are semi supported

Created by: eileenmcnaughton

Actually WRT hooks - they are supported to the extent they will continue to exist - but the code around them might change

Created by: eileenmcnaughton

SO @totten just focussing on the 'upstream / unlikely to change' - ideally they would each be listed & link to something that lists supported methods....

FWIW, the sentences seem fairly straightforward to me. Maybe there would be some mileage in using recommended, in place of / in addition to supported. How about:

"There are a number of ways in which an extension could interact with core. However, not all of them are recommended. For example, you should try avoid the use of BAOs and SQL statements whenever possible. The following are the recommend ways of interacting with core. Sticking to these methods increases the likelihood your extension will survive upgrades and be compatible with other extensions."

Created by: seancolsen

Just want to give this one another nudge here since it's been sitting in the queue a while. I haven't taken the time to read over everything right now, so apologies that this comment is pretty generic. What are the next steps to getting this merged. Who are we waiting on?

@eileenmcnaughton @totten

Created by: seancolsen

Hey folks is there something we can do to get this merged and then sort out more of the small details in subsequent work? Just trying to keep the PR queue looking fresh.

@eileenmcnaughton @totten

Created by: eileenmcnaughton

lets's merge & alter with follow ups

Created by: seancolsen

Ok super. Merging.