Problem/Motivation

There are several small problems with the FlagServiceInterface documentation. Classes aren't fully namespaced. Only brief descriptions are provided. In general, the documentation needs a bit of love.

Proposed resolution

Update and expand the documentation for FlagServiceInterface.

Remaining tasks

Create, test patch.

User interface changes

None.

API changes

Only documentation changes.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

socketwench’s picture

Title: Give the FlagServiceInterface some love » Give the FlagServiceInterface documentation some love
socketwench’s picture

Status: Active » Needs review
FileSize
4.94 KB
joachim’s picture

Looks good to me.

Is any further work needed here, or should this be committed?

socketwench’s picture

Status: Needs review » Needs work

I think the methods could use further description and examples.

martin107’s picture

Status: Needs work » Needs review
FileSize
636 bytes

Other patches have been improving the dockBlocks

Instead of rerolling, it is better to restart with a much smaller patch - only the FlagServiceInterface::unflag() param tags need FQDNames

I think the methods could use further description and examples.

The descriptions have been updated in the meantime - so the work that remains is a light sprinkling of examples.

martin107’s picture

... a light sprinkling of examples.

It is difficult to provide anything more than a trivial example for FlagServiceInterface::flag() without inserting code snippet the size of "War & Peace".

To criticize by own work ... hmm If I were looking for code example my instinct would not be to look in the dockBlock for flag()
So it is the opposite of discoverable.

So this is a question for review.... Should I move the code example? Perhaps Readme.md ?

joachim’s picture

I wouldn't put developer docs that are fairly detailed in README. If anything, that level of detail should go in docs pages on d.org (which are a mess at the moment, and still waiting for the tools to make them not be a mess).

If anything, our services are a main part of our API, so docs there are fine. Though I'm not sure we need to recap how to create a node programmatically -- if anything, that's an extra maintenance burden, as it's not our code we're demonstrating.

socketwench’s picture

I think we need to avoid "create the universe first" examples. We can cut those down by assuming that the nodes/flags/etc. already exist and reference the appropriate load() methods. Then, the reader would have a searchable reference elsewhere in Drupal docs to put together the bigger picture.

Even so, I still think we should keep the example about how to programmatically create a Flag! Let's just move it to FlagInterface where it'll be more useful.

Here's some off-the-cuff examples for the rest of the FlagServiceInterface. We will need to tweak the wording a bit more, I think.

martin107’s picture

Thanks for the constructive criticism ...

https://www.drupal.org/node/2126285

That goes through lots of my concerns.

Using the appropriate load and moving stuff into FlagInterface ...it is a good idea.
All the changes in #8 look perfect ... thanks for showing me how it is done.
socketwench++

  • socketwench authored a2c5599 on 8.x-4.x
    Issue #2482577 by socketwench, martin107: Give the FlagServiceInterface...
socketwench’s picture

Status: Needs review » Fixed

Thanks everyone!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.