The current workflow for documentation of core API changes involves redundant posting in multiple places, is not easily tracked for updates, and does not utilize well the existing Drupal infrastructure. We can easily improve on this situation as indicated below.

As I understand it, the current workflow involves these steps:

  • Submit an issue in the D7 issue queue.
  • Resolve the issue.
  • Submit a patch that the testbot is happy with.
  • Commit change to core.
  • Reference the change in the chronological log (the top portion of http://drupal.org/update/modules/6/7) and document the change in the bottom portion of the same page.
  • Cross-reference the change in the categorical page (http://drupal.org/node/394070). (New this year)
  • Add an issue to the Coder queue under the "Upgrade Routines" component. (New this month)

The last two steps are new to the workflow this year, initially to assist the Coder module developers. In addition, the categorical page provides everyone with a topical list of the changes.

Problems with the current workflow include:

  • Revisions to an issue are not easily tracked in the details or on the categorical page. If the issue is updated, then there is no mechanism to alert the Coder developers of this, or anyone else interested in the change.
  • The log page is one big ugly scroll.
  • The work flow does not take advantage of the existing module facilities (e.g. Project or Views).

To improve on this process (beginning with the D8 changes), we can:

  • Create a "placeholder" project for the API changes related to core.
  • Once a change is committed to core, add an issue to the API changes module with the details. This is identical to the current work flow except the information is posted in a different place.
  • Tag the issue based on Unstable release number and the categories it applies to (in the same way done on the categorical page).
  • Change the Coder Review and Upgrade flags as rules and routines are written.
  • Questions and clarifications about the change could be posted as comments on the issue. (This could significantly improve the post as with comments on the api.drupal.org site.)
  • If the change is later modified, reset the Coder Review and Upgrade flags to reflect this.
  • Use views to create the current chronological and categorical documentation pages from the individual issues. The chronological page will be limited to the log at the top of the current page, with each item linking to the individual issue page containing the details. This eliminates two of the current cross-reference steps (adding a Coder module issue and the categorical page item).

Please offer your comments and suggestions for modification.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

rfay’s picture

This sounds like a huge step in the right direction.

I've long been bothered by "needs documentation" tagged onto a bug after the fact.

pwolanin’s picture

Almost anything would be better than the current catch-as-catch and unclear issue statuses and editing of the gigantic changes node.

Probably too late for D7 to adopt this, but a good time for D8. A minor concern - we will at least need a placeholder node in the book hierarchy if this will mostly be Views - or perhaps the project itself could named be something like modules_7_8 and the project node could be in the book?

dww’s picture

Great suggestion to start thinking about improving this workflow. While we're brainstorming, why use "issues" for this at all? Are each API change really something that has a status that needs to be tracked, someone assigned to work on them, etc?

Why don't we just add a new "Core API change" node type?

We can use one of the existing taxonomy vocabularies to keep track of which version of core the API change belongs to (either "Drupal version" or "Core compatibility").

We can make all sorts of nice views of the API changes in various ways.

We could use a node reference to point to the UNSTABLE version that the change was introduced in (and on those release nodes, we could embed a view of all API change nodes that reference it).

We could have a node reference (probably multi-valued) pointing to the issue where the change was introduced. Issues could automatically have a block that link to any API changes that reference a given issue.

...

I just don't quite see why these need to be issues. Seems like the whole system would be a lot cleaner if it was a separate node type optimized for what we need, instead of trying to shoe-horn our needs into something that's designed to solve a different problem.

Anyway, kudos for opening the discussion!

Cheers,
-Derek

dww’s picture

p.s. we could certainly do fancy things via drupalorg.module's hook_nodeapi() so that every API change node that's submitted automatically opens an issue in the coder issue queue or something, if that's desired. The API change nodes could also node reference the issue where the coder support is written...

solotandem’s picture

dww, I agree with your points. I suggested a conservative approach not knowing how easy or difficult it is to make infrastructure changes.

drumm’s picture

One thing I've been wanting to add to API module for awhile is checks for accuracy. Like do @params match the function signature. Unfortunately, this is lower on the hierarchy of needs than things like parsing objects. (That is moving along, slowly.)

dww’s picture

rfay’s picture

Priority: Normal » Critical

I think this is an absolutely critical discussion. It's do-able, and offers a way for us to significantly improve our workflow/process.

Right now, documentation and other reviews don't get properly done (sometimes forever). Things get committed and then left open with "needs documentation". But there's no clue in the issue what documentation is needed.

Some things I'd like to see in our workflow:

  • An issue somewhere in the issue (or in a linked node?) that explains exactly what the issue is about. When the issue is committed, the summary needs to be correct and update. It should say what the issue was about, and what changed as a result, and give links to related resources.
  • Checkboxes for documentation, usability, security before a patch can be committed.
  • Perhaps an extra step in the workflow (between RTBC and commit) where these things can be addressed.

I propose a BoF at Drupalcon SF to discuss this and come up with an implementation plan. Who can come?

rfay’s picture

Related issue: #569552: Provide a mechanism for issue meta discussions.

Talked with dww in IRC: We're definitely going to do something about this at Drupalcon.

catch’s picture

subscribing, ideas here look good.

rfay’s picture

dww and I talked about this a bit. I dummied up a proof of concept at
http://d6.project.drupal.org/node/557272

It provides and issue summary (as a linked node) and various checkoffs, and related ssues.

We can do lots fancier than this, and we can do more, but it's pretty easy to do this much.

CCK is required for this, though. But we should get that soon on d.o.

dww’s picture

Note: from my perspective, rfay is confusing this with #569552: Provide a mechanism for issue meta discussions. ;) What he ended up doing at http://d6.project.drupal.org/node/557272 and http://d6.project.drupal.org/node/643648 is really for #569552. It's drifted away from what we're talking about here. It's still interesting and potentially useful, but it's not at all a proof of concept about the kind of workflow changes I proposed at #3 and other folks have since agreed was worth trying.

This issue is *specifically* about killing the hacky way that updates to http://drupal.org/update/modules/6/7 (or, really, the forthcoming http://drupal.org/update/modules/7/8 page) are integrated into the core development process.

To reiterate: my proposal is that http://drupal.org/update/modules/7/8 is a view of nodes of type "Drupal core change" (or something). This node type would have fields like:

-- required --
Title [text field]
Body [text area]
Related issues [node ref]
Drupal core version (one of our existing taxonomy vocabs will do)

-- optional --
previous version code sample [text area]
current version code sample [text area]
...

Sure, the issue summary stuff rfay is talking about could have a checkbox (better yet, a node reference) for linking to a "Drupal core change" node. But, that issue summary approach doesn't solve this particular problem.

I almost want to move this to the documentation queue, since this is really about a new core development workflow to make this documentation task easier.

David_Rothstein’s picture

I just read through this issue and wonder if there might be a simpler and more direct solution to the underlying problem?

As far as I see it, the underlying problem is that API changes don't get documented until it's too late - i.e. after the patch that changed the API has already been committed to core. This makes them invisible as part of our normal issue workflow, plus removes most of the incentive for people to actually work on documenting them.

In D6, the exact same problem existed with hook documentation - and consequently, hooks were not well documented. Then in D7, hook documentation was moved into *.api.php files in core itself (#314870: UNSTABLE-4 blocker: Add api.php for every core module), and the documentation improved overwhelmingly. It meant that patch reviewers and core committers could review the documentation at the same time as reviewing the patch, and consequently, patches which add new hooks now do not get committed until and unless they come with good documentation.

So, couldn't we just do something similar with API change documentation? Potentially this could even be done in D7... it would involve some work to get started, but nothing that is theoretically too late to do before the D7 release. Basically, we would need to move the entire contents of http://drupal.org/update/modules/6/7 and http://drupal.org/update/theme/6/7 into PHPDoc format in appropriate *.php files directly in the D7 codebase, and then potentially make any necessary changes to the API module (I don't know if there would even be any) to allow it to pick this stuff up. So the end result would be that the list of API changes would live on api.drupal.org rather than drupal.org, and their contents would be auto-generated from the codebase.

That doesn't entirely address the Coder module part of this issue, but I think it would still help a lot, by providing more standard ways (e.g. CVS commit logs) to see when and how API changes are occurring in core. In the long term (Drupal 8) I could even imagine the Coder Upgrade rules being stored in the Drupal codebase itself? - i.e., if you write a patch for core that changes the API, then instead of just writing a PHPDoc textual description of the change in the *.php file, you would also be expected to write a Coder upgrade routine for it, the code for which would live in the *.php file as well, although only the Coder module itself would ever execute it.

effulgentsia’s picture

subscribe

yoroy’s picture

subscribe. #651484: Enable CCK and node_reference is postponed on this

rfay’s picture

OK, a simple proposal for this one:

Create an API change node type:

  • Descriptive body
  • Documentation changes required (spelled out, for copy-and-paste to where they need to go)
  • Description of changes required to existing code
  • Links to the issue(s) that compose the API change.
  • Link to the related Coder Review issue.

If done correctly, we can make views that basically accomplish the objectives outlined by solotandem initially.

This is simple, and requires hardly anything besides a slight workflow change: API change issues can't go RTBC until the work has been done on the API change node.

dww’s picture

Before we introduce a new node type on d.o, we should discuss David_Rothstein's proposal in #13. I think that's the better solution, but I'm not sure if there are any objections to that.

Having *.api-change.php files in core that document any API changes from the previous version of core in a structured way seems like a huge win. I agree we could still do it for D7 if we wanted to. One of the first things we'd do after branching a major version of core would be to delete all those files from HEAD to start with a clean slate for the next version. ;) Plus, everyone would have all this documentation offline with them whenever they had a copy of core checked out (or ideally a core repo cloned, one we're on Git). Seems like doing all this doc work as part of the initial patch would actually be less work for everyone involved and we'd be sure it was getting done. In a weird way, this is also almost a solution for issue summaries from #569552: Provide a mechanism for issue meta discussions. ;)

Some specific questions if we go this route with PHPdoc comments:

A) Not all API changes affect single functions, some of our upgrade docs reference a set of changes than spans multiple functions (e.g. FormAPI, DBTNG, etc). How would those work via PHPdoc? I guess we could use defgroup for that, with the defgroup body as the main text, and then any functions touched by that change are tagged with that group.

B) What do we do when a single function is touched by multiple API changes in the same major version of core? I'm pretty sure this happens (although I don't have an example on the tip of my fingers). Can a function be in multiple doc groups?

rfay’s picture

I'm certainly open to #13.

Related issue: #100680: [meta] Make API module generate Form API documentation, where we're talking about taking the exact same approach for another bothersome area, consolidated documentation of large arrays.

I'm not sure if #13 might turn out to be unwieldy due to issues 17A and B, though. In D7, we put API changes in... then changed the API changes... then in some cases completely moved them to something else.

For example, our big changes splitting up hook_user, for example. You could call it one single change, or you could call it a pile of changes.

@drumm, could you chime in on this approach?

drumm’s picture

Don't put this on #100680: [meta] Make API module generate Form API documentation, that is about array data structures. With some effort, we can hang doc comments on any PHP syntax. Globals is an example of something a little non-standard that we document. Array elements will be another once #100680: [meta] Make API module generate Form API documentation is resolved. Maybe you want to stick doc comments in coder upgrade?

At a micro level, there are a couple things API module may provide in the future:
- Automatic API-level diffs. We have a big database of code, we can figure out what names, signatures, and docs have changed, disappeared, or have been added.
- @deprecated blocks for the trailing version, http://www.stack.nl/~dimitri/doxygen/commands.html#cmddeprecated
- @note, @par could be used to establish a convention for upgrade notes, http://www.stack.nl/~dimitri/doxygen/commands.html#cmdnote
- Anything else at http://www.stack.nl/~dimitri/doxygen/commands.html, stuff recognized by IDEs, programming conventions elsewhere, or conventions we are really confident about establishing, is fair game.

This is all inline with existing documentation. Separate files can be done, if you have a plan for what to actually put in them.

Macro-level changes- I don't have any great ideas offhand. Don't over-engineer it.

rfay’s picture

@drumm: Just for the record, I was saying that they are related approaches to different problems, so the thinking about one might affect the thinking about the other. I wasn't saying they had anything directly to do with each other. Both try to take our doc management and push it into code and have the API module (or something like it) manage the presentation of the results.

pwolanin’s picture

@dww, @David - in theory I like David's suggestion - in practice I think it would be a huge barrier to getting things done. So, I'd like us start with hte simpler suggesitons here, and ideally enforce that for the rest of the D7 cycle.

dww’s picture

@pwolanin: can you explain why you think "it would be a huge barrier to getting things done"? The work all needs to happen sooner or later, in my experience it's extra work to coordinate handbook changes + issues for this -- twiddling issue status and tags, etc. It's not like every core issue creates an API change, either. Sure, the pedantic core review process will tend to obsess about any line in a core patch, so it means all this text will get subjected to the inquisition. We could either attempt to relax a bit, or we could accept it as a feature that the upgrade docs will be well-written and clear as a result...

That said, I'm not attached to either approach. I think either one would be a big improvement over what we have now. I'm just trying to get as clear as possible on what the objections really are.

Thanks,
-Derek

rfay’s picture

The one (large) problem I see with #13 is that it relies on a whole development effort that has yet to be done, including development of a format for the changes and development on the API module to parse the new type of file, etc. In other words, instead of just rolling out a very drupalicious and simple solution to the problem, we have to do a whole bunch of arguing and then a bunch of developing (which nobody has signed up for).

@pwolanin, can you be more specific about your objections?

pwolanin’s picture

Can we please start doing #3 asap and let it evolve if needed into something more thorough? We've had 6 months of poorly documented core changes that could have been made better.

rfay’s picture

Can everybody live with #3 (perhaps modified by #16)?

pwolanin’s picture

@dww - clear and well-written upgrade docs are certainly to be desired, but there are many more people who can help write/edit a node than are likely to participate in a patch review.

rfay’s picture

@pwolanin: That's an important point: Accessibility and appropriate ease of change.

  • Only a small percentage participate in patching (or even reading a patch) but more of us follow issues. And even after the fact
  • After a commit, an API change description may need to be updated to explain its repercussions, etc.. And that shouldn't have to require a core commit.

The in-code technique, while it has great advantages, is also cumbersome for all and inaccessible to many.

There are many, many pieces of doc in our code that I would just update if they were editable, but I only sometimes get up the energy to create a patch to fix them.

David_Rothstein’s picture

Well, what we're talking about here is heavily technical developer documentation - i.e., API documentation. I think all other precedents for this do involve putting that kind of doc in the codebase.

Let me take a look at this over the weekend. I'm pretty sure that this can be done quite easily, with no patches to API module at all. (There certainly might be some patches we can think of that would improve it, but I don't see why we can't do the basics of what we're trying to achieve with API module as it is now.) I actually think my proposal is the simplest one in this issue - whether it's the best, that's another question :) All I'm really suggesting for starters is copying and pasting the existing docs from d.o. into the existing *.api.php files in the codebase, and adding some @code and @defgroup tags around them.

Re the questions in #17, I'm not even sure if we'd need that for a first pass (although it certainly might be a good idea in the long run), but I do believe it's possible for the same function to live in multiple doc groups. For example, currently, these two groups both link to hook_node_access():

http://api.drupal.org/api/group/node_access/7
http://api.drupal.org/api/group/hooks/7

catch’s picture

The current method, as horrible to deal with as it is, does allow for accounting to changes to APIs within major releases - e.g. UNSTABLE-3 splits hook_user() to hook_user_$op(), then UNSTABLE-7 changes the api for hook_user_load(). Do we just not worry about HEAD-HEAD API changes? Or rely on version control? If the answer to those questions is "yes of course" then I'm fine with it.

While I share pwolanin's concerns on api-change.php I think that's more a general issue that our core issue process has got ever more nitpicky, since people don't actually need to test patches any more. I don't think that should necessarily hold this up though.

For sweeping changes (dbtng, field API), those need their own handbook sections anyway - they're not possible to cover in update/modules, and they also split between individual functions, defgroups and the handbook. I think api-change.php would work for simple things like "the arguments to db_query() have changed and don't use it for string building or update/delete/insert", then leave in-depth docs for the handbook.

The only way I can see handbook-style documentation getting into core is if the new help system patch eventually gets in, and that's a year or two off now.

David_Rothstein’s picture

Do we just not worry about HEAD-HEAD API changes? Or rely on version control? If the answer to those questions is "yes of course" then I'm fine with it.

Yes, of course! :) I actually think version control is a great way to do this. If you're following along with HEAD, you can just run a diff between e.g. UNSTABLE-3 and UNSTABLE-4 and filter it to only show the changes to the api-change.php files, which will immediately and unambiguously tell you the new API changes as well as any alterations to the existing ones that were made between those releases.

It seems like the best way to get at this data. Whereas now, in theory we do split out the docs page by release, but people go back and edit old sections all the time, move things around, etc - I don't particularly trust that any section on that page actually gives an accurate description of what changed in that unstable release.

For sweeping changes (dbtng, field API), those need their own handbook sections anyway - they're not possible to cover in update/modules, and they also split between individual functions, defgroups and the handbook. I think api-change.php would work for simple things like "the arguments to db_query() have changed and don't use it for string building or update/delete/insert", then leave in-depth docs for the handbook.

Yes, agreed completely. I think we should copy exactly what's in the upgrade docs now. So in cases where they link to an outside handbook page, we should copy the link, but not the content of the handbook page itself.

David_Rothstein’s picture

OK, I worked on this a bit over the weekend and here's what I came up with. To see what things would look like, I took a few API change examples from the categorical list at http://drupal.org/node/394070 and moved them to PHPDoc in code. I tried two methods:

  1. In the first (simple) method I just ported the text as simply as possible and allowed the API module to reconstruct a single page where the API changes are automatically organized by category. This worked mostly great, although it turns out it does still require a small patch to the API module in order to be able to reproduce the section headers (this just uses standard PHPDoc tags from http://www.doxygen.nl/commands.html). I attached the core patch, a screenshot, and the API module patch below.
  2. The second method tries to go a little more in the direction where we might eventually head: I used @defgroups to put the API changes in different categories, so that each API change would get its own page but also show up in listings. This would require more work in the API module to support; basically, we'd need to add another type of entity ("API change") that the module knows about and can look for and display differently. For the purposes of trying it out now, I just gave each API change a dummy "function" so that the API module would recognize it. I attached the core patch and screenshots of three different pages below.

I think we should seriously consider going with option #1 for now. It solves most of the problems in this issue, as well as others, and I think putting this on api.drupal.org is the way to go in the long term. We'd need to get the small API module patch committed and then finish copying the existing docs into code, but that's pretty much it. At that point, all API changes could be documented through the normal core process and tracked in the issue queue via standard methods.

David_Rothstein’s picture

Project: Drupal.org infrastructure » Drupal core
Version: » 7.x-dev
Component: Other » documentation

Moving this to the core queue for review. If we decide not to go with this approach, we can definitely move the issue back to d.o. infrastructure.

moshe weitzman’s picture

#32 is really sweet. thanks for the efforts, David. Yeah, #1 looks like a great improvement for now.

system.api-change.php in that patch is already getting a little hard to read because it so laden with doxygen and so free of php. its gonna get harder as these files grow. we might want to consider separate files for each change . we would use a new subdir like system/api-change/user_permissions.php. This structure is similar to the .test files. IMO, small files would be easier to edit. not a big deal though.

rfay’s picture

Title: Improve the workflow for documentation of core API changes » Document API changes as part of the development process

#32:

  • It works
  • It adds discipline to our process
  • The only negative I see is that the changes are not really available to people who don't read patches. I'd prefer that this information were available by simply clicking a link or something. But I can definitely live with this.

#33: I definitely agree about using multiple files, one for each change, named with a naming convention we could come up with.

All of this does not solve the coder problem, and since we highly value coder, we should probably figure out a way to deal with that also. Spinning that problem off into #827578: Create coder updates (when APIs change) as part of the development process.

Changing the title to indicate what this issue has really become.

jhodgdon’s picture

Status: Needs review » Needs work

What problem in the current "system" for recording updates is this proposal really going to solve?

As I see it, there are two problems:
a) The module update guides as Handbook pages are very big and disorganized, so a module developer has no way of grasping what they need to do to fix up their module.
b) The module update guides don't always get sections added to them when they should.

I don't think the proposal in #31 solves either of these two problems:
(a) You are either going to have a big changes.api.php file or a big directory of small files, instead of a big Handbook page. That's not an improvement.
(b) Nothing is going to prevent patches going in that don't include an entry in this file, any more than patches are currently prevented from going in that don't include proper documentation, proper review, etc.

Also... Fundamentally, I don't think that Drupal (or any other project) should keep a record of its past history in the source code repository, except in the CVS/Git history. If we start this now, think about what this history would look like by the time we get to Drupal version 10 - it would be a complete mess.

catch’s picture

Priority: Critical » Normal

This isn't a release blocker.

rfay’s picture

@jhodgdon, #35: IMO the biggest problem is that API change docs don't get done as a part of the development process. They're an afterthought, and the overall implications are not explained in issue either. This approach allows us to make the documentation process part of the dev process, which I think is a big +1. There are other ways of doing it, but this is one of them.

jhodgdon’s picture

Right, that was (b) in #35 - that the updates don't get done. API documentation also doesn't get done, although it's right there with the function in the code base. Putting this mess into the code base isn't going to help it get done, in my opinion.

And it *is* a mess. It really doesn't belong in the code base at all, in my opinion.

jhodgdon’s picture

Here's an alternative proposal: Have every commit that involves an API update have a particular hash tag (or other indicator) in it. Then have a way to list out those commits, which would then lead people to the issues involved.

This would be much cleaner.

For instance, every API change would have a commit message like:

#987652 API-CHANGE by rfay: Update the foo API so that it works with bar transactions.

dww’s picture

Status: Needs work » Needs review

@jhodgdon: Thanks for raising your concerns. A few responses:

- Re:

If we start this now, think about what this history would look like by the time we get to Drupal version 10 - it would be a complete mess.

You must have missed this sentence in my comment #17:

"One of the first things we'd do after branching a major version of core would be to delete all those files from HEAD to start with a clean slate for the next version."

Thanks to the magic of version control, all we ever maintain in any given branch of core are the docs for API changes from the previous version.

- Re:

(a) You are either going to have a big changes.api.php file or a big directory of small files, instead of a big Handbook page. That's not an improvement.

That's where api.d.o comes in. No one but core developers are really expected to read these files as files in the source code (and even then, not necessarily). Instead, they'll just go to a nicely formatted set of pages on api.d.o (or their own offline api.module site). However, instead of the clumsy approach of trying to understand the edits to the list by pouring through the revisions tab on a monster handbook page with hundreds of revisions, you can just use our revision control system to understand when/why changes were edited. We don't have to worry about the problems of multiple people trying to edit a single handbook page simultaneously and losing work. Instead of a giant free-form text area where people have to correctly format things manually and via convention, we'll have structured PHPDoc comments with tags that people will have to follow.

- Re:

(b) Nothing is going to prevent patches going in that don't include an entry in this file, any more than patches are currently prevented from going in that don't include proper documentation, proper review, etc.

Well, but that's a big barrier. ;) Our pedantic police squad for core patches is pretty mighty, and growing. It's becoming basically impossible to get core patches committed that haven't gone through the fine-tooth-comb for proper punctuation and grammar of every phpdoc comment. That's a good thing, but it means that if we institute a policy that for the patches that actually change an API in some way the patch is expected to document the API change from the previous version, I have a high degree of confidence that we'll prevent the API change patches from landing until the upgrade docs are written, clear, and properly formatted.

All that said, I'm still not completely attached to this approach. I'd also be happy with the new "API Change" node type from comments #3 and #12. If we don't converge on a consensus that #13/#31 is the right way to go here, I'd be happy to fall back to #3/#12 and try that as a more structured alternative to One Giant Handbook Page that would still keep all this outside of the codebase and patch/review process.

p.s. I'm not sure why this is "needs work". There's nothing concrete that needs to happen before more reviews would be valuable. We need to decide if #13/#31 is the right approach or not, and I think it's important for those reviews to continue before we invest any more time in either approach.

David_Rothstein’s picture

Agreed with most of what @rfay and @dww wrote - just adding a couple things:

  1. a) The module update guides as Handbook pages are very big and disorganized, so a module developer has no way of grasping what they need to do to fix up their module.

    The approach here will directly fix the "disorganized" problem. It doesn't fix the "big", although I think that's less important once things are better organized. Also, the second approach listed in #31 would directly fix the "big", and I think that's where we might be able to head in the long term (although not now).

  2. (b) Nothing is going to prevent patches going in that don't include an entry in this file, any more than patches are currently prevented from going in that don't include proper documentation, proper review, etc.

    I think the evidence suggests it will get a LOT better. Look at what happened after #314870: UNSTABLE-4 blocker: Add api.php for every core module went in. New hooks added in D7 are getting documented much more extensively than ever before, and in general, the API documentation in Drupal 7 is much more complete and higher quality (a large part of which is due to your work and your patch reviews :) whereas the documentation at http://drupal.org/update/modules/6/7 is much less complete and much lower quality, because people do it as an afterthought and it rarely gets reviewed by anyone else.

    It won't fix things 100% and there will definitely still be omissions that slip through, but the point is that those errors and omissions can then be treated as bug reports and fixed via the normal process, just like any other API documentation bugs. By putting update documentation in the codebase, we would be giving it higher visibility and making a statement that it is important as any other API documentation (which I think it is).

  3. Here's an alternative proposal: Have every commit that involves an API update have a particular hash tag (or other indicator) in it. Then have a way to list out those commits, which would then lead people to the issues involved.

    I think this is not necessarily an alternative proposal; it could actually be complementary. But I'm not sure it provides too much benefit over what can be achieved with issue tags?

David_Rothstein’s picture

By the way, regarding @moshe's suggestion of using multiple files (rather than a single api-change.php file) I think that might not be possible right now, because with the current proposal it is all just one big @defgroup - and I don't think there is a way to break the @defgroup documentation itself up into multiple files?

Again, this is something option 2 from #31 could address in the long term, though, since it allows things to naturally be broken up over as many files as desired.

Even all in one big api-change.php file, though, I think editing sample code snippets and other technical documentation is way way easier in a text file than it is in a node body on drupal.org :)

rfay’s picture

@jhodgdon: We respect your opinions (negative in #35 about #31). But we're looking for a specific workable solution that is not too hard to implement. I think David Rothstein's #31 and my #16 are the two basic proposals that have any structure to them.

Do you have an alternate proposal that would actually solve most of the problems? Or are you in favor of something like #16 instead of #31?

I think that a key part of any of these is that our development expectation must change. It must be unacceptable to get a patch to RTBC without taking care of the API change docs that go with it. #31 fits that into the process (assuming that we all behave and that committers insist). Our experience with hook documentation using this technique has in fact been very positive. And we're hoping that a technique like this would fix the Form API Reference.

I encourage you to propose something that you think solves the problem better than a variant of #31.

jhodgdon’s picture

It's not that I'm objecting to the proposals per se.

It's that I don't think that changing the standard for how to document API changes is not going to solve the problem. We already have a standard, and it's really not all that difficult to carry out (edit the massive API change page). Tweaking the standard so it can go into a patch instead of a Handbook page, to me, is like rearranging the deck chairs on the Titanic. My reasoning:

a) The current, massive API change page is nearly useless. Big changes, like "Oh, we totally changed how database queries are done" and small changes like "We got rid of the 5th parameter to obscure function foo()" get equal weight on that page, and you just simply couldn't read the whole page and get any perspective on what changed between 6/7, or any idea of where to start in modifying your module. I don't see any of the proposals could do much to change that -- we'll still have a massive amount of documentation on what changed. Although I do agree, having this on api.drupal.org, so that it could link to the relevant v-old and v-new functions (and be auto-linked-to from them), would be a plus.

b) You need to get buy-in from Dries an whoever is the maintainer of D8 in order to solve the other part of the problem: that the standards and review process are not always followed, because patches are RTBCd and/or committed that don't follow the rules.

Note: These roughly follow (a) and (b) in #35 above... just more specific.

As far as the specific proposals go, I think the approach of #31 is on the right track... I have some specific thoughts and suggestions about how to refine it and what the goals should be:

1) I'm in favor of having this end up on api.drupal.org and writing the doc in some kind of text file(s) we can patch. It will require some additions to the API module. That's OK, but someone has to write them, of course.

2) There needs to be a way to link to functions/classes/constants/etc. across different versions of Drupal displayed on api.drupal.org, and preferably, automatic back-links. So we probably need a new tag that says "Make this a link, and on the referenced thing's page, link back here" and "This is the version it refers to. Perhaps:

* @ref truncate_utf8() DRUPAL--6

And have the API module do the rest (automatically generating the links in both directions, figuring out the URLs, etc.). The backlinks would go into a section entitled "API Changes DRUPAL--6/DRUPAL--7" or something like that?

3) Make it so you can just add each API change docblock patch to the end of a particular file. Use the group/topic mechanism we have now to do the grouping and listing, rather than having to go up to the top of a file to insert a change, or figuring out where in the file the thing should go. E.g. within your API change docblock, just do:
@ingroup api-changes-file
@ingroup api-changes-form
etc. and make this automatically group the stuff together.

4) Don't introduce false syntax like:

+function permissionsRestrictAccess() { }

This is not a function, it's some kind of a header.

Probably each API change should be a docblock starting with /** and ending with */, but not followed by an artificial function foo(). I think the API module can be modified to figure this out -- we already have docblocks without things after them (@defgroup and @file blocks, for example). Make this a new type, maybe @api-change.

5) OK, this is turning into something more concrete in my head...

/**
 * @api-change unique-identifier-for-this-change One line descripton of this change.
 *
 * Descriptive information goes here, with D6 and D7 code examples, much like what's on the current API 6/7 page.
 *
 * See also @link another-change-unique-id this other change doc @endlink.
 *
 * @ref hook_perm DRUPAL--6
 * @ref hook_permission DRUPAL--7
 *
 * @ingroup api-changes-hooks
 * @ingroup api-changes-user-module
 */

Results:
- Both the "Hook changes Drupal6/Drupal7" and the "User module changes Drupal6/Drupal7" pages would contain a link to this API change docblock, and clicking on the link would display it on its own page.
- The D6 hook_perm page and the D7 hook_permission would have a link to this API change docblock, in the "Changes Drupal6/Drupal7" section on both pages (generated automatically).

rfay’s picture

Looking forward to getting this active again as hopefully we have some time available now.

Here is my rant on improving the issue queue which I'll keep ranting about.

jhodgdon’s picture

Title: Document API changes as part of the development process » Make API changes in Drupal core be nodes

Here's a concrete proposal that rfay and I have been talking about:

a) Each time a change in Drupal core affected module or theme developers (i.e. an API change), we would create an "API Change" node on drupal.org.

b) The "API Change" node would have the following fields (besides title):
- Versions -- some way to specify that this is a change between Drupal 6.x and 7.x, or 7.2 and 7.3, so we need to have a from/to version field.
- Affects -- a way to say this affects themes, modules, or both
- A category (or multiple) -- this would be similar to the categories on the current Categorical module/theme 6/7 update pages
- An issue (noderef) -- or multiple issues
- The node body would be a summary of the change, including what the developer of a theme/module would need to change
- The commit date (or dates) when the change was added to Drupal

c) Instead of the huge and nearly useless Module Update 6/7 and Theme Update 6/7 pages that we have now, we'd have a View with exposed filters, which would allow you to list the changes by date, by category, by version, etc. and sort by those fields

d) Instead of rfay manually sending out API change notices, people could subscribe to API change notice nodes.

See also #569552: Provide a mechanism for issue meta discussions. Having issue summaries (as discussed in this issue) would be useful for figuring out what to put in these issue summary nodes.

dww’s picture

#46 sounds identical (if more detailed) to my proposal in #3. I'm therefore all in favor. ;) Seems like the proposal to do this as a file in core itself was stalled due to the problems of trying to introduce such a change into core itself. So, at least making the "handbook" on d.o for this not so terrible is a lot easier to solve and implement.

If we're happy with #46, next step is for someone to click together a feature (yes, as in feature.module) for this and post it as a tarball or patch against drupalorg.module. Then we can deploy it on a d.o test site and if we're happy, put it live.

Cheers,
-Derek

jhodgdon’s picture

Does the feature just need to include the content type and the view, or does it need to do something about subscriptions and permissions too? [I'm not completely familiar with Features]

What versions of CCK/Views are you using on d.o?

dww’s picture

Views 2 for now. Ain't got no version of CCK yet, but let's assume we'll start with 6.x-2.8 unless we need 6.x-2.x-dev for some reason.

Initially we only need the content type and view. We can add more features to the feature in follow-up issues. ;)

Cheers,
-Derek

David_Rothstein’s picture

Seems like the proposal to do this as a file in core itself was stalled due to the problems of trying to introduce such a change into core itself.

Not really - the patch in #31 (the first patch) is both working and workable. All that remains is to actually copy the rest of the documentation over. And it paves the way to be able to eventually do something like #44 in the future.

The only reason I stopped working on it was that it sounded like there was still discussion and I didn't want to spend several tedious hours copying/reformatting the entire documentation into a PHP file if it wasn't going to wind up getting committed at the end :) I'm still 100% willing to do this, though. I think it would only take a few hours of work to reach that stage.

If people would rather go another way that's fine with me too. But in the long run I do think API documentation belongs on api.drupal.org and example PHP code belongs in the codebase (rather than in CCK text fields) :)

David_Rothstein’s picture

In any case, the patch to api.module itself in #31 shouldn't get lost: It needs its own issue in the api.module queue.

I've rerolled it now and posted it here: #996242: Support @ref, @section and @subsection tags

jhodgdon’s picture

I don't think we want to save each change, fully documented, in the core CVS repostitory. Imagine the clutter this would contribute by the time 9.x is out...

As far as putting these nodes on api.drupal.org vs. drupal.org, I'm undecided. Logically, right now the 6/7 update pages are in the Module Developers Guide.

dww’s picture

@jhodgdon: We shouldn't care about lots of revisions in CVS/Git. And remember that I proposed one of the first things we do when we branch for a new version of core is clear out this file in the new development branch. So, e.g. when the 7.x branch of core is finally made and we've got a branch for 8.x, the copy of the file in 8.x would be pruned. Therefore, there's no additional clutter by the time we're working on 9.x. Sure, this file will have a lot of revisions, but so does node.module. ;) Who cares?

Overall, I think it makes more sense to keep this stuff in the code with the rest of core, just like the *.api.php files. Also makes it possible to realize your dream that patches aren't committed until they document their own API changes in the upgrade docs, since changes to the api-changes.php file would be included along with the changes to the rest of the code in the same patch file.

That said, the fact that there have been so many API changes between 6.x and 7.x means having a single file is going to run into the same problems of being overwhelming and hard to make sense of -- a problem the existing handbook page suffers from. So, the main counter-argument that makes much sense to me, why a view would be better, is that it'd be a bit easier to automate having a summary of changes that link to the details.

And having said that, I think either approach here is better than what we have now, and I'd be happy to see either one move forward... ;)

jhodgdon’s picture

I'm not worried about having lots of revisions. I'm worried about what I think is David's proposal: having each API change add a file to core (or a section of a file) that would explain the change. We have CVS/git commit messages so you can see what has been revised in the file... I don't think we additionally want to have a huge page with detailed explainations of the changes there in core.

And as you noted, one of the big problems with the current "system" of making one big file for all the API changes between 6 and 7 is that the page is HUGE and it is hard to find stuff in it. Making it into a bunch of nodes and a view would potentially make it so that people could actually find information on the page. Hopefully.

Also, after talking to webchick over the weekend, it's clear to me that we aren't going to make it so that patches don't get committed until their API change is documented. Non-starter, too much of a burden on core devs.

rfay’s picture

Oh dear. Drupal 8 is moving right along, and we're going to behind the 8-ball (haha) on this one.

We now have CCK on Drupal.org. Why don't we just make a content type and start handling changes that way, even if we don't have the full UI worked out? Or if we're going to do something else, let's do it.

catch’s picture

Sounds like a good plan.

jhodgdon’s picture

I would be willing to do that, but is this another case of "Prairie is going to address this"? I would like to avoid putting in a lot of effort and then having the solution postponed because of Prairie.

rfay’s picture

IMO, Prairie is going to address how things are presented, and how the user interacts with them.

This is a case of us needing to deal with the *data*. We need to create appropriate data and present it as best we can. And then prairie can use that data in a different way, or present it differently.

I can nearly guarantee you that Prairie will not deal with these data issues if we don't already have them, and that it will help present them better if we already have the data.

jhodgdon’s picture

OK. If we're doing this as nodes, here is a summary gleaned from all the proposals above (and perhaps some new thoughts) of what the fields need to be. Note that this would make it work for the Theme 6/7 as well as Module 6/7 update guide:

- Title
- Description of change (node body) including examples [I think this is better than having separate fields for the examples, because this way they are integrated into the flow of the body]
- noderef links to originating issue(s) - multiple valued
- Branch committed to (e.g. 8.x) [use existing Taxonomy - Drupal version or Core compat?]
- Branch it's a change from (e.g. 7.x) [same as committed to taxonomy]
- Affects ("theme", "module", "profile" -- allow one or more to be selected)
- Date committed [multiple valued in case there were multiple commits]
- Component (same as Drupal Core issue queue)
- noderef links to Coder issues? [does this need to be coder review vs. coder upgrade - two separate fields?]
- Has it been added to Coder Review? [or should this just be known by having no noderef value for the coder issues?]
- Has it been added to Coder Upgrade? [ditto]

Note: One thing that was proposed was to have a field giving which release it was included in (unstable 1, alpha 3, etc.). But how do we know what the release is going to be, since the release hasn't happened yet when the API change is committed? We don't want to have to go back and update a bunch of issues with the release... maybe this can be automatically known from the date committed field?

rfay’s picture

Thanks, @jhodgdon.

I think we could include the *commit* it was included in.

One thing that I just realized: Throughout D7 we were not only trying to track API changes against D6, but also API changes against D7 itself. As we all know, it was rough just keeping track of HEAD. So I'd like to see us figure out a way to do both of these. Do you think it would be possible to have both a "What changed vs previous stable" (D7 in this case) and also "What changed against what we did have here". I'd love to see us handle that somehow.

jhodgdon’s picture

Hmmm. Maybe we can think of a scheme for making the branch to/from fields do something about head to head changes? I'm not sure how that would work. I guess if branch to/from were both 8.x, then it would be clear it was a head to head change and not so much a 7->8 change?

But we'd also need to be careful about situations where issue A's commit changed function foo()'s signature/behavior from 7.x to 8.x, and then issue B changed it again head-to-head, so we would want to go back to the issue A change node and update it with issue B. We have that problem now in the huge 6/7 updates page -- you have to go back and change code examples so they use new syntax from time to time.

What to do?

rfay’s picture

Should we have a "Change from previous major" field and a multivalued "Change from current major" field?

catch’s picture

But we'd also need to be careful about situations where issue A's commit changed function foo()'s signature/behavior from 7.x to 8.x, and then issue B changed it again head-to-head, so we would want to go back to the issue A change node and update it with issue B. We have that problem now in the huge 6/7 updates page -- you have to go back and change code examples so they use new syntax from time to time.

This is going to be painful to keep track of whatever we do - could we maybe add a function/class/method field to this? That would help to pick up on function name changes, and you could then list changes by function name, which might help to discover older changes being changed again. Could also potentially be auto-linked to api.drupal.org

dww’s picture

@jhodgdon re: Prairie: Sorry about the other issue. However, I can safely say that this issue is not on the radar of the prairie initiative at all. It's also fairly self-contained. The other issue was really a bandaid for "I can't figure out WTF is going on with this issue!", which is a vastly bigger problem with lots of possible solutions. This problem here is more limited in scope, and I feel a lot safer just designing and implementing something sane and rolling with it. If/when something better materializes, we can always adapt. I don't know why, but this seems totally different to me.

Anyway, I'll stake some of my reputation on the line here and declare that no one should say "let's wait and see what Prairie comes up with for this problem before we deploy something."... ;)

Cheers,
-Derek

drumm’s picture

Agreed with dww.

Everything prairie wants will have to be broken up into doable issues, get done, and be deployed; all incrementally. With the redesign out of the way, no more monolithic projects should be started. Do collaborate, but don't make impossibly big pile-ups.

jhodgdon’s picture

#62 - I'm not following your suggestion rfay or how that would help or what it would do?

#etc. - ok on prairie not usurping this, good.

So as a practical matter, what do we need to do? Make a content type with CCK fields, make a view, and get them onto the live site how? I know we can do views by just exporting, but not sure about cck-field-containing content types?

dww’s picture

Status: Needs review » Needs work

Re: practical matters: We're trying to standardize on using features for this sort of thing. So, someone(s) should click this together on a dev site (e.g. jhodgdon.redesign.devdrupal.org), then turn it into a feature (including at least the content type, field definitions, and views), then post the feature here (either as a standalone tarball, or as a patch to drupalorg.module). Ultimately, the feature is going to get checked into Git and deployed/maintained as part of drupalorg, but don't let the patch creation aspect slow you down. So long as it's named something like drupalorg_apichanges (more or less) it's just as easy to review as a tarball or a patch. Hope that helps...

Re: the spec in #59: I don't think we want a manually-entered date field, do we? The commits associated with the issues referenced by the API change node can all be computed automatically (and displayed in a block or something) -- seems pointless (and error-prone) to try to record that manually. Plus, we don't yet have date on d.o -- just cck core and node references. We probably could add all of date, too, but unless there's a compelling reason, I'd prefer not to introduce another dependency. See the Drupal.org development guidelines for more...

Also, in terms of the taxonomy, I guess I have a slight preference for using the "core compatibility" vocab so it's easier to associate these API change nodes with the release nodes of core. But, I can't really explain why I think that'll matter. ;) Fundamentally, I wish we didn't have two separate vocabs for this, but that's a whole other can of worms. However, the deeper problem here is that you can't have two "taxonomy fields" that point to the same vocab without custom code for it (at least not in D6). If we're only using core's taxonomy system (I spit on thee!) then there's no way to do both "to" and a "from" field. OTOH, we don't *really* need to record both. We never document N -> N+2 API changes -- it's always N -> N+1. So, we could just record the version of core that's being documented (e.g. N+1) and implicitly know it's a change from the previous (e.g. N) version. That doesn't work for using these two fields to record HEAD->HEAD, but maybe we just want a "is a HEAD to HEAD change" checkbox to handle the case where the two fields would point to the same version. I guess if we want maximum flexibility, we add another checkbox for "Is a change from the prior version". Then, we just have the taxonomy for the current version (e.g. 8.x), and two checkboxes which would mean "changed from an older version of 8.x" and "changed from 7.x"...

Finally, seems this needs work -- there's no proposed solution ready to review and test yet...

Thanks!
-Derek

jhodgdon’s picture

OK - roger on Feature-making.

Regarding dates...

What I think we really want to know is not the commit date actually, but the first release (alpha2, beta1, 8.3, etc.) the change went into. We might not know that when the API change node is first created, because we don't actually know what the next release is going to be called (e.g. it could be UNSTABLE10, or maybe it's time to do ALPHA1).

After the fact, the (involved, annoying) way I normally figure this out when writing a section of the 6/7 module update guide page is:
- Look through the issue to where webchick or dries made a comment saying they committed something, and note the date. Hopefully there is only one, but that's not always the case.
- Look through the list of 7.x release nodes and find the next release after that date.

That "process" would obviously not work well in code. So how can we get that information reliably, and what should the field on the API change node be to record that information? A noderef to a release node (in which case it would be blank until the next release has been created? A Git commit ID or git commit URL? Something else? I was thinking of the commit date because then this could be used to figure out what the next release after that date was, but there are probably other options...

sun’s picture

Is there any particular reason for doing this as a separate node type?

I would have assumed that directly "attaching" this information to issue nodes would be more natural and also streamline the process for everyone involved. It would, potentially, also open the doors to provide this facility to all drupal.org projects (contrib), even including feature/topic forks/branches of Drupal core.

dww’s picture

@sun re #69: Comment #3 explains why separate nodes for this are better than trying to do this directly via issues. Although some of the details of my original proposal have since been refined, the basic arguments are still valid.

@jhodgdon re #68: A couple of ideas spring to mind:

A) project_release already knows how to order releases properly, not just chronological or alphabetical. So, we could record the currently available release at the time the change is made as the "From release" and then fill in the "To release" offline via a drush command or something that runs periodically (or even whenever new releases are packaged) to search for all API change nodes that don't have a "To release" but where there's been a newer release since the "From release". Or something. ;)

B) project_release + project_issue could have better support for making "milestones" as release nodes that haven't been released yet. E.g. have a "next release on branch X" release node before you're ready to release. But, it'd be available as a version in the issue queue and in the node ref for these API change nodes. Since it's just a release node, the nid isn't going to change, even if the title does. So, you could just make an "8.x-next" release node that wasn't actually a packaged release yet (since there was no Git tag for it), etc, etc. Big can of worms, and will probably slow this down if we block on that, but perhaps the "right" way to solve this (and a host of other problems). See #135800: Milestone management for more.

Probably (A) is better to pursue for now. There might be something even easier, but it's not coming to my (tired) mind at this time...

catch’s picture

At least two API changes are in 8.x now, http://drupal.org/node/1152742 was just posted.

jhodgdon’s picture

I've just proposed a standard for issue summaries, which has some bearing on this discussion:
http://drupal.org/node/1155816

The idea would be that after an API change node was created,it would have checkboxes such as:
- Updated the theming guide
- Updated the module dev tutorial
- Updated the API tutorials on Drupal.org
- Updated the Examples for Developers project
- Added to Coder
etc. Some or all of these might be be yes/not yet/not needed radios rather than checkboxes?

And as a note, these API change notifications might apply to both module and theme changes -- so maybe have a field that says "affects themes" "affects modules", with the opportunity to select both?

rfay’s picture

The one thing here: It's not just API changes that need checkboxes/gates of this type. It's new features and lots of other things. Is it right to tie the checkbox gates to API changes? Or maybe this should be called "API changes and new features" or something.

Actually, one of the oddest things about the classic Pro Drupal 7 Development is that it (mostly) only added material on existing stuff, and missed some really key new features. We don't want to do that.

jhodgdon’s picture

Hmm.

So maybe we need this stuff on all issues? In which case, maybe it should just be part of the Issue Summary, and hence on the Issue node itself? Making a separate node for "what is updated" might not make sense, if it's needed for every issue that is committed.

rfay’s picture

An API change is a special type of documentation, but perhaps it's not unique. Perhaps it's just a field itself on a more general documentation element.

jhodgdon’s picture

OK, let's step back a bit and think about

a) Issues in RTBC/fixed status need to have summaries, in order for them to be understandable (for committers and for people who later just want to see what the issue was about). Summaries should include a description of what has changed in the module API, theme API, UI, and paths. I think this information should be on the issue (for example, it could be CCK fields on the Issue node, or it could be a comment, or who knows).

b) We need a way for module developers to be able to figure out what has changed between version X and version Y of Drupal that might affect them. The current 6/7 update page is not great for that, but is one example of how it can be done. If the API change information is a CCK field as part of the issue summary, perhaps we can just use Views to make a view that would show the changes based on your choice of filters? Or we could make "changes" be separate nodes. I'm not sure what that would really buy us at this point, since I think it would just duplicate the information that is already going to be (hopefully) part of the issue summary?

c) We also need a way for the online doc team to realize that module/theme APIs as well as UIs and paths have changed, and to track whether these changes have been put into the online doc. One way to do this would be to add some checkboxes or radio buttons to the issue (or change node) that would say things like:
- Theme guide on drupal.org updated (yes, no, not necessary)
- Module developer guide and API tutorial section on drupal.org updated (yes, no, not necessary)
- Examples for Developers project updated (Y/N..)
- Coder updated
- Other documentation updates done (Yes, no, not necessary) with a text field to explain what those other updates are

The more I think about this, the more I think it all belongs on the Issue as the fundamental node type that would have all the needed information. In which case, we might need a new issue state, like:
committed (documentation incomplete)
which would track whether one of the items in the list on (c) was pending.

And a UI note: I would propose putting all this new Issue information into collapsed fieldsets, turning on the ability for everyone to edit the Issue node, and tracking revisions on Issues. So we would have new fieldsets for:
Issue Summary (see http://drupal.org/node/1155816 for details of what the fields would be in that fieldset)
Issue Progress (this would have some fields around the current status of the issue as it is in work, before it's ready to commit)
Documentation Needs (or some header like that - item (c) above)

Thoughts?

jhodgdon’s picture

Issue progress stuff is on http://groups.drupal.org/node/136174

sun’s picture

@jhodgdon: That's more or less exactly what I was thinking. In 99% of all cases, the issue itself that introduced a change is the ideal hook to capture the information. I'm also still on the fence why this entire facility should only be available for Drupal core and not for similarly sized projects like Views and such.

However, @dww still has a point in that #3 contains some wish-list items that might not be resolved so easily when being attached to an issue. For example, in terms of API changes, the current core development workflow is to potentially backport changes to earlier versions on the very same issue, which means that the assigned version would no longer be correct. Of course, there's always room to change current development procedures in light of good arguments and reasons, and this particular topic is currently discussed anyway, but yeah, needs analysis.

solotandem’s picture

Responding to Jennifer's "the more I think about this" point in #76:

I disagree with the idea of not having a new node type to track API changes. If the primary use case is "We need a way for module developers [and themers] to be able to figure out what has changed between version X and version Y of Drupal that might affect them," then a new node type seems essential. The API change node type will:

- include a useful (to the developer or themer) description of the API change
- include status codes (as mentioned previously on this issue)
- omit the long discussions on issue queues, but have references to all of them (most will not care to nor should have need to read them)
- combine multiple issues into a single, more cohesive and understandable "API change"
- allow for tracking changes to API changes (e.g. the registry reversion) (this is one of the weakest features of the 6/7 update style page)
- allow for more useful summaries using Views with various filters, etc.

dww’s picture

My concerns with #76:

A) Issues are already complicated and overwhelming. I just want to describe a bug, please. ;) If we add all this stuff, we're turning this form into (even more of) a gigantic monster of knobs and fields. I predict this will be a big step away from the goal of making the issue queues less intimidating. It might be slick and nice for the doc team, and a small handful of the hard-core inner circle that deals with the pain of core development (I include myself in there), but for the other 99% of the people, it's just going to make a bad situation worse.

B) Issues are sort of like commits. We want to keep them (ideally) small, focused, discrete, etc. "Commit early, commit often" also applies to the queue: keep things small and manageable, and create lots of them. However, the API change documentation should be like release notes, not a river of commits. This should be the curated list of things people *actually* have to care about. (Side note: I can't say it enough: I hate it when people just take the output of my helper script to get a list of all the commits and submit that as-is as their release notes... makes me wish I never wrote that script in the first place). A given API change might involve many separate issues. I don't want to see all that noise. I just want to know what ended up changing, what I have to do to my code to get it working with the new version, and have a way to read all the gory details if I really need to. If we force ourselves to tie this documentation directly into issues, we're doomed to end up with a fire-hose of little changes that people have to pour through.

C) Yeah: issue workflows. Changing versions. Changing titles. Moving projects. Issues are rather chaotic (which is natural in an open source project like this). People do weird things to issues all the time. I think it's a good thing is there's a bit of a barrier between participating in the issue tracker and editing the API change documentation. I'm not saying we should make it hard. I'm just saying they shouldn't automatically be the same thing. I doubt random new users are going to be trying to update the API change nodes unless they actually mean to.

D) I'm worried that we're trying to solve too many sort-of-related problems all at once with one solution. Basically, right now on d.o we have a pretty small number of tools: issue nodes, issue tags, book nodes, project nodes, release nodes, forum nodes, and g.d.o groups. We have lots of problems that aren't well handled by that combination of tools. I'd prefer we started adding more tools that solve specific problems well, instead of just trying to add more and more functionality to the existing tools. We have a pain point with documenting changes -- let's build something that does that well. I know the pace of change on d.o makes it seem like "well, if we're going to try to change *anything* we might as well attempt to solve as many problems as we can since who knows if we'll ever get another chance", but I'd like to get us out of that mentality. If we can build and retain some momentum on improving our tools, I think we can look forward to a lot of changes, large and small, hopefully all for the better.

Cheers,
-Derek

gdd’s picture

I tend to agree with dww, especially on points #A and D. We should make small iterative changes as we can. Making the issue form have even more fields is not helpful. I also tend to think that API changes need to be a new node type. Often API changes will span across several issues and being able to aggregate that history seems essential. Also all the points in C and that sun made are highly relevant here.

So +1 to node type.

jhodgdon’s picture

OK, I am being convinced that API change nodes need to be separate nodes. We still need to have issue summaries somehow; will continue to discuss that on
#1036132: Provide a mechanism for issue summaries

Meanwhile, my dev-staging site is up and running again, and I'm going to take a go at making an API change node type and some views.

jhodgdon’s picture

Status: Needs work » Needs review
FileSize
4.06 KB

I have a first stab at API change nodes built on http://jhodgdon-drupal.redesign.devdrupal.org:
- I turned on two CCK modules: field group and number. Also Views UI.
- Added a new content type and added fields to it
- Updated permissions so that any logged-in user can create/edit these nodes
- Created two sample nodes from entries in http://drupal.org/update/modules/6/7
- Added a View that lists the changes, to replace http://drupal.org/update/modules/6/7
- Exported the content type, fields, permissions, and view to a Feature (attached)

You can see the view in action at:
http://jhodgdon-drupal.redesign.devdrupal.org/list-changes
Note: I ran cron but the search index doesn't seem to be built, so the search terms field doesn't work.
And it's a bit crude, first pass, etc.

If you'd like to be able to log into the site to create Change record nodes, etc., find me in IRC and I can reset your password on the site to something memorable.

Comments welcome...

dww’s picture

Very quick glance: looks great! Rock onward. I'll have no time until mid next week at the earliest to look at the implementation itself, but seems pretty straight-forward. I don't anticipate any major hurdles. I'd say you should just keep running with this, flesh it out, sand off any rough edges, and we should be good-to-deploy soon.

Thanks!
-Derek

jhodgdon’s picture

[Edited May 17 - added links and crossed out the part I implemented]

Cool! So I'd really like it if a few people could do a quick look:
a) Look at the views page and see if it would meet your needs to find API change nodes of interest http://jhodgdon-drupal.redesign.devdrupal.org/list-changes
b) Look at a change node item and see if it needs any adjustments to the fields http://jhodgdon-drupal.redesign.devdrupal.org/node/1156203
c) Try creating a change node item and see if the UI is OK http://jhodgdon-drupal.redesign.devdrupal.org/node/add/changenotice

You will need to get a login to my dev site to do #3, which if you have shell access to the staging server and and your own dev site, you know that you can get my master password and use that to log into your account -- so feel free to do that. Otherwise, if you ping me in IRC or use my d.o contact form, I can reset your account password. The contact form is probably better on weekends as I'm unlikely to be on IRC on the weekend, sorry!

Think about:
- Missing fields?
- Non-needed fields?
- Better field labels or field help?
- Name of the content type and view, and page title?
- Things that should be changed from text field to drop down or vice versa?
- Order of the fields on content type and view?
etc.


===> Oh, I just realized the View needs exposed filters to find items that haven't been added to coder yet, haven't been added to examples yet, etc. The content type has those fields, so it would be only a few minutes to add them to the View. But I'll wait until there is more feedback.

rfay’s picture

Well I like it. And if we want incremental change from here, we can get it. This is (more than) what I was hoping for. Thanks!

jhodgdon’s picture

That's one vote, any others? I still need to add a couple of exposed filters to the View (see #85 at bottom). Any other suggestions?

solotandem’s picture

I think you have a good start. Things that struck me are:

Branch and version fields
- could these be select fields?

Field labels
- "Updates done"
-- the word "done" assumes that while the radio values allow for other states -- remove "done"
-- the word "update[d|s]" on the individual fiels is redundant with the fieldset title
-- Theme guide "on drupal.org" updated -- are the quoted words needed? -- if so, then what about documentation and examples project?

Coder
- please separate the Coder update field into Coder Review and Coder Upgrade as they are two separate beasts with their own statuses

Is it worth indicating where in the book outline to update the theme and module developer guides? Especially, if there may be multiple places to update? Or, if the change needs a new book page? So, this could be a conditional text field with multiple values that displays if changes are indicated?

Another way to look at it is a cross reference to the developer guides (similar to the issue links) for those who want to know what to do and don't need to read all of the issue history. These would be multi-valued node reference fields.

jhodgdon’s picture

Thanks for the review!

Branch and version fields as selects - I don't know how to make that happen easily, given that this could be used for contrib modules as well as core.

Regarding "Is it worth indicating where in the book outline to update the theme and module developer guides?" -- I already have a text area there where you can put in details of what needs to be updated (at the bottom of the Updates section). I think that will be enough.

Other suggestions: I'm going to make a 2nd pass shortly and will incorporate.

jhodgdon’s picture

I've made some updates based on feedback from solotandem. Attached is a new Feature (so is there a way to make Features remember what I've exported the last time, so I don't have to fill in the form the next time? I'm not a Features expert...).

Feedback welcome -- see #85 for details on what to look at (I'll edit to add URLs right after I post this).

rfay’s picture

@jhodgdon, use drush features-update (drush fu)

jhodgdon’s picture

Shouldn't that be in the UI for Features too?

yoroy’s picture

Issue tags: +prairie

House-keeping & reading up.

catch’s picture

Cross posting #1160886: Figure out documentation of API additions and changes in minor releases of 7.x - API changes and additions to 7.x now that it's out need to fit in here somewhere, they can't be avoided entirely.

jhodgdon’s picture

Bump... Can we please get some reviews here so we can RTBC this and get it live on d.o? Or fix it so it will be worthy of RTBC?

See #85 for details on how to review, and #90 for a patch (well, technically it's a feature and not a patch).

webchick’s picture

Spent about a half hour reviewing this. This is going to be an awesome improvement! Notes:

Here's a review of the node/add form:

1: Default "project" to "Drupal core" [done] since it's the one used in 99% of cases.
2: Remove "from" branch/version; the one it's introduced in is enough info to derive the one it isn't.
3: Change "to" to "Branch/version change was introduced" (or something)
4: Move "description" up since it's a required field
5: Collapse the "Updates" fieldset and name it something so the people updating that will know they need to do it; this is too overwhelming for devs to know what to do with, and I worry they won't bother filling out the form.
6: Ditch the autocomplete title on issues, make it whatever you and cweagans were talking about
7: Coder upgrade/review should probably default to "Not done yet"
8 And all the docs ones to "Not necessary"
9: Under the "Description" field, I would love to see a link to an example of the depth of text we're expecitng.

Thanks!

jhodgdon’s picture

RE #96 - thanks!!!

Progress report:
#1 - done, deployed on demo site
#2 - done, deployed on demo site [btw: reasoning was you only need to know the branch/version where the change was introduced, because the previous version is the one where it wasn't there]
#3 - done, deployed on demo site [feedback welcome on the text]
#4 - done, deployed on demo site
#5 - done, deployed on demo site
#6 - Just a note that the idea here is that cweagans is adding a CCK field formatter to the Project Issue module, so we can enter an issue node ID in a numeric field, and it will be formatted just like if you type [ # 12234 ] (without the spaces) in the body of any node or comment on d.o currently. Then we'll change the Issues field to be a multi-valued integer field instead of a node reference (the node ref field doesn't really work now as there can be multiple issues with the same title and anyway who thinks about issue titles?!? we just use the node IDs).
#7/#8 - I've changed this extensively. We now have a field that says "who does this impact" and then the updates done fieldset is collapsed and is just simple on/off checkboxes saying "done". There will be a separate view to find needed updates.
#9 - done (for now this is just linking to sections of the module/theme 6/7 update guides, because this is what we have as far as good examples).

None of these have been reflected in the view yet. To be done... but all are in the node/cck definitions...

jhodgdon’s picture

I've now updated the View. I split it into two pages:
http://jhodgdon-drupal.redesign.devdrupal.org/list-changes - for module/theme developers to find relevant changes
http://jhodgdon-drupal.redesign.devdrupal.org/list-change-updates - for docs team, coder team, examples team, etc. to find api change nodes that haven't yet been documented in the online doc, coder, examples project, etc.

Comments?

webchick’s picture

One other note, on http://jhodgdon-drupal.redesign.devdrupal.org/list-changes it'd be nice to be able to sort this list chronologically, for the benefit of people chasing HEAD, or the benefit of people looking to see how a release evolved. The current list "somewhat" has this, by virtue of the #s next to each change.

We talked in IRC and said neither node create date nor node update date were ideal, what would be ideal instead is the commit date of the change. Not sure if that's easy to pull in tho; would probably have to be manually entered.

jhodgdon’s picture

For now I can add this as a field. I think it makes sense to be commit dates, since often there are several patch commits involved in one change node.

Hmmmm...

See comment #67 above:
dww: "I don't think we want a manually-entered date field, do we? The commits associated with the issues referenced by the API change node can all be computed automatically (and displayed in a block or something) -- seems pointless (and error-prone) to try to record that manually. Plus, we don't yet have date on d.o -- just cck core and node references. We probably could add all of date, too, but unless there's a compelling reason, I'd prefer not to introduce another dependency. See the Drupal.org development guidelines for more..."

so I guess for the time being dates are a "not possible".

jhodgdon’s picture

dww: how can we compute the commits and display them automatically, and how can we incorporate this into the View where you search for api changes by date, and/or want to order them by date?

rfay’s picture

Status: Needs review » Reviewed & tested by the community

I'd be happy to go with this as it is now.

If I got my druthers I'd have the (multivalued) commit hashes that made up the change (and maybe even the associated comments that went with it?). I'd definitely like to have a complete issue reference (#33333: "required" string showing multiple times below module table rather than #33333: "required" string showing multiple times below module table

This does not have to be perfect. It can be iteratively improved if we even get it close to good enough. And it's close enough to good enough. Marking RTBC.

jhodgdon’s picture

Status: Reviewed & tested by the community » Needs work

There is one usability issue as it is now: the Issues field being an auto-complete noderef. The problem is that issue titles are not always unique, and it's also hard to locate them by title. I could make it a straight integer field now, but it wouldn't link to the issue. So I think this is "needs work" until we figure something better out. cweagans said he could have a patch to project issue in a day or two and mikey_p promised to get it in... we'll see.

Regarding commits... I'm still not sure whether the average dev knows about or can find commit hashes - I personally have no idea what they are -- if I were taking an issue and making a change node, where would I find the commit hash? And what would it do for me?

rfay’s picture

A commit hash turns into a nice link.

The hash 8a9f7b6d4a75e52327180ab3b10d2fd73471469b turns into
http://drupalcode.org/project/drupal.git/commit/8a9f7b6d4a75e52327180ab3...

which is as good as it gets. So I'm not saying we'd *present* it as commit hashes :-)

jhodgdon’s picture

Um. So
a) Where does an ordinary person like me find a commit hash to enter?
b) "turns into a link" - you mean there is a text filter to do so? That won't help if it is a separate CCK field that lists the commits. Is there a CCK formatter that turns a hash into a link?

dww’s picture

Yay for more progress here! We're getting close...

Re: versions vs. dates: see comments #68 and #70. We can't just have a node reference to the version where a change was introduced, since we don't necessarily have a release node for the release that introduces the change at the time we're creating these documentation nodes. Furthermore, we don't even know what the next release is going to be called, since it might be unstableN+1, or we might switch to alpha1, or it might be 7.5 or 7.6 depending on security updates, etc. I think #70A is probably our best bet. I haven't seen any alternative proposals that would work.

Re: #99 and ordering: Given that a particular change might have lots of commits with different dates, it seems rather ill-defined to sort these chronologically. Frankly, I wouldn't spend much time worrying about that. Seems like a can of worms that's likely to delay this for everyone to try to support an edge case (chasing HEAD and needing to see these changes "in order"). If anything, let's punt this and open a follow-up issue about it if it seems like an important edge case to handle.

Re: commit hashes: In theory, all commits for a given issue use #xxxxxx to link back to the issue. So, #493074: Back-link to the commit as a comment on the related issue. would be code in versioncontrol_project that parses commit messages looking for these references and creating a table of commit <=> issue nid mappings (or something). See also #443000: When viewing an issue, display a list of commits that reference that issue # for additional background. There's no reason humans should do this work manually, we should just automate it once. Not only would it allow nice things like an auto-generated block/list of commits when you're viewing an issue, it'd allow us to do smart things here (and probably other cool features, too). This aspect could also be punted to a follow-up issue (postponed on #493074). Meanwhile, we just leave it out. So long as these nodes have node references pointing to the issue(s) where the change was made, we can go back later and provide an automated list of Git commits once #493074 is done. You could argue that not every single commit referencing any issue linked to the API change node is actually related to the API change itself. Perhaps the API change we're documenting is but one small piece of a bigger issue. In that case, maybe you do want a field to directly specify commits that actually implemented the change. But, that again seems like overkill and making this more complicated than it needs to be. I think it's fair to just make the simplifying assumption that all commits that reference issues linked to these nodes are relevant to the API change. Then we don't have to have extra fields for any of this, humans never have to enter this data (making these nodes easier to create) and we can still (eventually) provide nice display options to see all the relevant commits.

Re: The UX problems of node references, titles and nid integers: Instead of #1175704: Add CCK field widget and formatter for issue references I think #243543: nid text field widget? would be a vastly better solution.

Cheers,
-Derek

solotandem’s picture

Check out the "View commits" link on any project page (at bottom of right column). Each commit listed has the hash and URL that rfay is referring to,

webchick’s picture

On the create date thing, I still do feel it's really important to show that information. Otherwise I have no idea what's new since the last time I looked at the list. I'm less fussed about actual commit dates; that can be derived through other means.

dww’s picture

@webchick Re: #108: Then I propose we just use the post date on the API change node itself to record the date the change was "made". In practice, I think that's going to be close enough to satisfy the basic requirements, and I'm guessing most of the people actually authoring and updating these API change nodes are going to have sufficient powers to alter this date if needed.

jhodgdon’s picture

OK...

#106 (first paragraph) brings up a good point: the reason I had "from version" in there before was because you don't necessarily know what the next version will be called. So for instance if the currently out version is UNSTABLE-8, is the next one UNSTABLE-9, or is it ALPHA-1? Who knows, at the time of creation of the API change node. I forgot that when I got rid of that field. I think we can leave out the "previous branch" field, but we should have a field where we can record the currently-out version that doesn't have the API change.

RE - node ref field vs. issue ref field - formatting it like #648218: Make API changes in Drupal core be nodes seems better to me than formatting it like http://drupal.org/node/648218 -- right? And thanks for finding cweagan's issue #1175704: Add CCK field widget and formatter for issue references and linking it here...

jhodgdon’s picture

RE #107 - there are a *lot* of commits to Drupal Core. If I am creating an API change node, I don't think I want to page through them to find the particular commits related to the issue. Something automated like dww is talking about could be added later (if it can be automated).

webchick’s picture

<completely, 100% off-topic>
What we ideally want is something like this:

Commit messages interspersed with comments when they happened

That'd make it easy to enter the commit hash there.
</completely, 100% off-topic>

rfay’s picture

I'm just saying that any old hash can be turned into a link using the standard views technique rewriting...

The hash should be entered by looking up the commit where the API change was made.

We need to encourage the committers (OH PLEASE) to paste the hash of their commit into the issue. Or do fancy like suggested in #112. But just a paste by the committers would get us going.

jhodgdon’s picture

So paste the commit hash into the issue. But do we need it on the API change node too? Seems like extra work for the person making the API change node, and if it's on the issue, someone who wants it can find it there?

If it's just for the issue, let's not get this discussion sidetracked here.

dww’s picture

@webchick #112: That's exactly what #493074: Back-link to the commit as a comment on the related issue. is about. Feel free to post your image there if you want to propose that as the specifics of the UI. ;)

@rfay et al: Please read #106 where I talk about "Re: commit hashes". It's pointless for humans to enter this information into API change nodes, even in the short term. It's also pointless to ask over-worked core maintainers to do the work that a computer can easily automate for them. Down with manually pasting commit hashes anywhere. Up with #493074. ;)

Cheers,
-Derek

rfay’s picture

I'm all for do-able now approaches. I don't see anything except hope in #493074: Back-link to the commit as a comment on the related issue.. Pasting commit hashes is trivially easy. Today. I do it for every commit. git show HEAD. Copy. Paste. I'd rather have a filter that would turn it into a drupalcode.org link, but it's identifiable without detective work.

@jhodgdon, I would dearly have a link to the commit(s) in the API change node. But as I've said before, this doesn't have to be perfect. In our D7 mess it was nearly impossible to tie API change information to the commits, or to figure out how multiple commits could have affected the outcome.

jhodgdon’s picture

RE #116 - Forcing the person who creates the API change node to find the commit hash seems like a large burden (speaking for myself, and the current situation where you would need to go to the Drupal Core project page, find the commit there, etc., because it is not already displayed on the issue page). And if the commit *can* be found on every issue, then it doesn't seem an undue burden to have to click on the issue link(s) from the API page to go find the commit(s), because if you need that you will probably also need to read something on the issue (maybe anyway). So I would vote for not having this.

Now. One other 2-minute-to-build feature: Should we have a block that goes on an Issue page and displays the API change nodes that reference the issue?

rfay’s picture

One other 2-minute-to-build feature: Should we have a block that goes on an Issue page and displays the API change nodes that reference the issue?

Absolutely!

jhodgdon’s picture

OK.

So cweagans (yeah!) made a patch to project_issue that adds a formatter for issues. So this is now ready to go mostly, and that patch has been added to the dev site.
#1175704: Add CCK field widget and formatter for issue references

Still to do:
a) Block that can be displayed on an issue and show the API change nodes related to it
b) [new idea from IRC] - RSS feed for new API change nodes, so a dev can subscribe and see them (probably project-specific).

jhodgdon’s picture

c) [another thing missing, from webchick in IRC] Some sort of breadcrumb on the API change node that will lead you back to the listing page

jhodgdon’s picture

I've made some updates:

1) In place of cweagan's formatter patch, used mikey_p's widget-plus-formatter patch, from issue
#1175704: Add CCK field widget and formatter for issue references
I found one bug in it (see issue) but it is really great and cool and that bug should be easy enough to fix. This allows you to type in the issue number or the title, and it auto-completes with both the issue number and title showing, and then the formatter is as in cweagan's patch, so it's really great. See the results (of the formatter) at:
http://jhodgdon-drupal.redesign.devdrupal.org/node/1156203
And if you are logged in, you can also edit the node and see the widget in action as well.

2) Made a block for the Issue page that shows the related API change nodes. You can see this on:
http://jhodgdon-drupal.redesign.devdrupal.org/node/106192
Note: I didn't do any theming. It's just an HTML UL list, which bluecheese themes without bullets. I think it's probably fine.

3) Added an RSS feed for changes, by project. You can see this at:
http://jhodgdon-drupal.redesign.devdrupal.org/changes/drupal/rss.xml
I also added a change node for another project, so you can see its feed here:
http://jhodgdon-drupal.redesign.devdrupal.org/changes/search_by_page/rss...
(and you can verify there is no cross-contamination in the feeds.

I still need to figure out a breadcrumb...

jhodgdon’s picture

Status: Needs work » Needs review

OK, I'm a bit stuck on the breadcrumb.

I added a patch to the drupalorg module that gives the API change nodes a breadcrumb.

However, the drupalorg_crossite module does this:

function drupalorg_crosssite_preprocess_page(&$vars, $hook) {
  global $user;

  // In most cases, the navigation elements should provide orientatiuon within
  // the site. Breadcrumbs are usually not necessary.
  if (arg(0) !== 'admin') {
    $vars['breadcrumb'] = '';
  }
...

So the nice breadcrumb that I created is going away. Why should it do that? Anyway, getting a breadcrumb to display is not so easy with the drupalorg_crossite module...

Any ideas?

Other than that, I think this whole thing is good to go, and I think it's time for another review.

I'm also not sure how to deploy it. What this functionality needs:
- New content type and associated CCK fields
- View
- Block from view added to right sidebar of the Issue node page
- A couple of modules turned on (field group, number -- might not need number any more...).
- Updated permissions so that any logged-in user can create/edit these nodes
- Patch to the Project Issue module from[#1175704]
Most of that can go into a feature, but I'm not sure about all of it?

jhodgdon’s picture

FileSize
632 bytes

I made a bzr patch for drupalorg for the breadcrumb (which isn't showing up due to drupalorg_crossite, see comment above).

Status: Needs review » Needs work

The last submitted patch, 648218-drupalorg-bzr.patch, failed testing.

drumm’s picture

Looks like these are associated with projects, so we want to grab the navigation to match issues and everything else. I believe you want project_project_set_breadcrumb($project_node, TRUE). Drupalorg_crosssite already looks for project breadcrumbs, so it will help the navigation fall into place. The second argument can be used to put in more levels.

Hopefully you won't have to change this around, but may be useful to know the internals. In drupalorg_crosssite, calls to drupalorg_crossite_in_breadcrumb() use the breadcrumb to match up the section navigation. If $vars is passed to it, it does include the tail on the breadcrumb on the page; for example, this page's remaining breadcrumb is the "Issues" link above the node title.

jhodgdon’s picture

drumm: thanks! I'll try that out.

jhodgdon’s picture

So. Two more items remaining to make this really ready to go (IMO):
a) Test new patch from mikey_p on #1175704: Add CCK field widget and formatter for issue references
b) Try to fix the breadcrumb and nav with thoughts from comment #125.

jhodgdon’s picture

(a) is deployed - new patch seems to be fine.

drumm’s picture

Also, drupalorg_crosssite needs to know changenotice nodes are part of the downloads section. (No, I'm not proud of this area of drupalorg_crosssite code, but refactoring is another issue.)

jhodgdon’s picture

RE #125...

I changed the nodeapi($op = alter) to do this:

     $proj_nid = $node->field_project[0]['nid'];
      $proj_node = node_load($proj_nid);
      $extra = array();
      $extra[] = l($node->field_project[0]['safe']['title'], 'node/' . $proj_ni\
d);
      $extra[] = l(t('Change records'), 'list-changes', array('query' => array(\
'project' => $proj_nid)));
      project_project_set_breadcrumb($proj_node, $extra);

This produced an OK breadcrumb for a non-core API change node, but the core one got a strange link to http://jhodgdon-drupal.redesign.devdrupal.org/project/drupal%20core in there.

I'm also not sure how I can tell drupalorg_crossite that api change nodes are part of the Downloads section, but I'll keep looking...

jhodgdon’s picture

Status: Needs work » Needs review
FileSize
2.39 KB

Ah, thanks drumm!

Here's a patch combining drumm's patch from #129 and my changes to drupalorg (patch generated from bzr diff), to fix the navigation.

Which is much better! See
http://jhodgdon-drupal.redesign.devdrupal.org/node/1156204 (Drupal Core api change node)
http://jhodgdon-drupal.redesign.devdrupal.org/node/1156205 (contrib api change node)

I think this is ready to go - more reviews?

Status: Needs review » Needs work

The last submitted patch, 648218-131-drupalorg.patch, failed testing.

jhodgdon’s picture

Status: Needs work » Needs review
FileSize
6.29 KB

Here's a feature with the latest config. I'm not sure what else we would need besides this feature, the patch in #131, and also mikey_p's patch from #1175704: Add CCK field widget and formatter for issue references.

The one thing that isn't in code that I can think of is that the block "Change records for issue page" needs to be added to the Right Column region. Probably it would be good for someone else on a different dev site to try adding the feature, adding the patches, and turning on the block, and seeing if it all works there?

rfay’s picture

Status: Needs review » Reviewed & tested by the community

I don't see how we can delay doing this any further. This solution is elegant, Drupalish, does exactly what we need, and is both extensible for the future *and* if it doesn't work out in some way can be redirected.

I'm quite sure we'll be improving this as we go, but it's a fine step forward.

I experimented by creating an API change with four related issues, which worked fine. It's easy enough to add the issues in using the autocomplete. I then visited the issues, like this one where the block with the change records shows on the right side, just like we've always needed. Finally, the Change record search page gives a nice views exposed filter to provide the granularity we need when searching.

This is the perfect start. Let's jump in.

jhodgdon’s picture

That's two votes... How do we get this deployed?

jhodgdon’s picture

One thing: on #653068: Update documentation is incomplete there was a bunch of ranting, but one constructive thing I noticed was a reminder about the categorical page: http://drupal.org/node/394070 ...

So maybe we need a taxonomy field for API change nodes [tagging or fixed], or a text select field [probably multi-valued], or some other way of categorizing them? The categories that are on the current Categorical List page (not sure who came up with them, or if they are useful, but here they are):

Module Info / Install
System
Permissions and Access
Database
Menu
Blocks
Comments
Input Sanitization and Input Formats
Taxonomy
JavaScript
CSS
Theming
Form API
File API
User API
Node API
Multilingual
Miscellaneous

Should we leave this as a separate issue or do this now?

catch’s picture

That looks a lot like the component list for core.

jhodgdon’s picture

Yeah, except just a little less coherent. :)

pillarsdotnet’s picture

webchick’s picture

Project: Drupal core » Drupal.org infrastructure
Version: 7.x-dev »
Component: documentation » Other
Issue tags: +needs drupal.org deployment

Tagging and moving to the right queue (I think).

jhodgdon’s picture

See comment #133 above for how to deploy.

Attached is a new feature zip, renamed as per requiest by drumm. I have also attached a screen shot of the features screen, so in case I need to do this again, I can cross-check which components are in the feature.

It would probably be good to try to deploy this on another test site, and maybe test it there, before d.o, but I suppose you know that drumm. :) I would be happy to run it through its paces on another test site and verify that it's all working the way I would expect.

jhodgdon’s picture

Also, it looks like the project issue patch is at "needs work" now, so I guess we can't deploy right now?
#1175704: Add CCK field widget and formatter for issue references

jhodgdon’s picture

IRC with drumm today... we think there are a couple more small things that should be done on this before deployment (and now we're waiting on the project issue patch anyway):

a) Make project be an argument instead of exposed filter on the list_changes page

b) On the project page, have a link in the sidebar to show the change nodes list for the project [drumm: project module has some trickery to do the links all over the project page, there is probably somewhere to alter in another link. But I'm not familiar with it.]

c) put the list_changes page into the navigation, similar to what was done for the individual change nodes

drumm did part of (c)

d) a note about block visibility -- the block from the view showing related change nodes for issues should be set to show only on node/* in the block admin when deploying

jhodgdon’s picture

note that drumm edited my drupalorg crossite module for (c) so it is in the diff (he made it so http://jhodgdon-drupal.redesign.devdrupal.org/list-changes?project=3060 thinks it is in the downloads section. The breadcrumb will highlight the right tab.)

so when I make the next bzr patch for that module, it will be there.

drumm’s picture

Status: Reviewed & tested by the community » Needs work
Issue tags: -needs drupal.org deployment
jhodgdon’s picture

drumm: I can't figure out how to tell the drupalorg_crossite module that list_changes/[project short name] should be part of the project nav. It's getting into the Downloads section, but it's not picking up the project. See
http://jhodgdon-drupal.redesign.devdrupal.org/list-changes/search_by_page
http://jhodgdon-drupal.redesign.devdrupal.org/list-changes/drupal

The individual change nodes are getting the right placement, due to a call to project_project_set_breadcrumb() in drupalorg_project.module / hook_nodeapi(), but not this views page. I suspect the same kind of thing needs to be done, but I'm not sure where/when?

jhodgdon’s picture

OK, I've made some changes on my devtestbox...

1) The change list has an argument (project short name) instead of an exposed filter for the project. So you can go to
http://jhodgdon-drupal.redesign.devdrupal.org/list-changes/search_by_page
http://jhodgdon-drupal.redesign.devdrupal.org/list-changes/drupal

2) The change list has a link from the Development block on the sidebar of the project page. You can see it here:
http://jhodgdon-drupal.redesign.devdrupal.org/project/drupal
(way at the bottom of the right sidebar).

3) Running the new patch for the widget/formatter for issue nodereferences from #1175704: Add CCK field widget and formatter for issue references

So... the navigation is not quite right still (see #146). I tried using hook_views_pre_render(), but it doesn't seem to get called, so I'll have to try something else tomorrow.

jhodgdon’s picture

Status: Needs work » Needs review

OK, I have the nav working (stupid typo fixed #146)! I think (again) that this is ready to go - just needs reviews...

Check out:
http://jhodgdon-drupal.redesign.devdrupal.org/list-changes/drupal [simpler change list page for module/theme maintainers (Drupal core)]
http://jhodgdon-drupal.redesign.devdrupal.org/list-change-updates/drupal [this is the complicated page for docs admins where they can figure out what needs documenting]
http://jhodgdon-drupal.redesign.devdrupal.org/list-changes/search_by_page [simpler page, on a different project]
http://jhodgdon-drupal.redesign.devdrupal.org/node/1156208 [a Drupal core change request]
http://jhodgdon-drupal.redesign.devdrupal.org/node/1156205 [a non-core change request]
http://jhodgdon-drupal.redesign.devdrupal.org/node/1051082 [an issue with the change requests sidebar]
http://jhodgdon-drupal.redesign.devdrupal.org/project/drupal [Drupal core project page, with new link to changes list at bottom of right sidebar in the Developer section]

Next comment will talk deployment and have attachments...

jhodgdon’s picture

Deployment:

a) Feature (attached below) needs to be installed.
b) drupalorg patch (attached below) needs to be applied and deployed.
c) mikey_p's patch from #1175704: Add CCK field widget and formatter for issue references needs to get added to project_issue and deployed.
d) The block "Change records for issue page" needs to be added to the Right Column region. It could be visible only for node/* (it goes on issue nodes).

Probably it would be good for someone else on a different dev site to try adding the feature, adding the patches, and turning on the block, and seeing if it all works there?

pillarsdotnet’s picture

Why are there separate columns for "Introduced in branch" and "introduced in version" when the version number includes the branch number?

jhodgdon’s picture

I guess we don't need the columns in the view. I think the filters are useful to have.

jhodgdon’s picture

OK. #1175704: Add CCK field widget and formatter for issue references has reached RTBC status... so can we consider making this one RTBC also, and is there anything else we need to do to deploy? See comment #149

stella’s picture

demo site looks great, big +1 from me

However, sometimes it's not possible to say whether a coder review/upgrade is possible or needed, so I'd be in favour of a N/A option, but would settle for the ability to mark it as done and add a tag so I could filter on them later if needs be. A comment would work, but then not filterable.

jhodgdon’s picture

Hooray! dww just committed the patch for #1175704: Add CCK field widget and formatter for issue references (it is set to Needs Work for tests only).

So #149 (b) is done, but the new code would need to be deployed to d.o.

RE #153 - My original version of this did have Yes/No/NA for each doc option, but webchick objected that it was overly complicated and made the Add form overly long.

So the plan for workflow currently is that after the issue is committed, the person creating this node marks it as "affects module devs" etc., and leaves all the doc/coder checkboxes at their default of "No".

Then the docs team, examples team, and coder team (who use the checkboxes), can filter on, for example, "Issue that affect module developers, where coder isn't done yet" using
http://jhodgdon-drupal.redesign.devdrupal.org/list-change-updates/drupal

If they determine that the doc/examples/coder changes are not necessary, they can just edit the API change node and mark it "Yes" for Coder Review is done, adding a comment to the doc notes section saying "Actually, we don't need a coder review update". Then it won't show up in the search the next time.

I think it will work out OK... we can try it for a while and if it's not meeting the needs of the doc, examples, and coder teams, we can definitely revise.

Right now, there are no tags or comments on API change nodes, by the way.

jhodgdon’s picture

So... I don't think there are now any barriers to getting this deployed...

I was just going to mark it RTBC, but there have been a couple of changes since the last time it was RTBC, and they probably should be reviewed (see comment #148 above for review links). The only thing that changed since the last time is navigation, and there was some refactoring in the patch to #1175704: Add CCK field widget and formatter for issue references (which didn't affect its functionality in any way).

New deployment instructions (revision of comment #149 above):
a) Feature (attached on #149) needs to be installed.
b) drupalorg patch (attached on #149) needs to be applied and deployed.
c) project_issue - version including the patch from #1175704: Add CCK field widget and formatter for issue references needs to be deployed.
d) The block "Change records for issue page" needs to be added to the Right Column region. It could be visible only for node/* (it goes on issue nodes).

dww’s picture

I'd love to deploy this, but I'm literally just about out the door for 2 weeks completely offline, so that'd be highly irresponsible of me... :(

That said, re: c) it's already committed and pushed. Someone just has to deploy from master (e.g. the 6.x-1.x-dev build). I've tested the changes mentioned and they're totally fine. So, there's nothing fancy in that part to deploy, just a standard project_issue jenkins deploy and bzr merge should do it.

Wish I could bring this over the finish line -- but I just don't have time tonight, and even if I did, all of us on the infra team have sworn never to deploy things and then be afk/MIA in case of trouble. ;) Sorry!

Thanks to everyone who helped on this, especially jhodgdon!

Cheers,
-Derek

webchick’s picture

Status: Needs review » Reviewed & tested by the community

Just a note that my comment about overwhelmingness of the "N/A" option was before that fieldset was hidden by default; now that it is, that part of the form could be optimized to the audience who needs it.

However, I agree that we should just go ahead and deploy this, and iterate on it more once we find out what the actual pain points are, so we can ditch http://drupal.org/node/1152742 as soon as possible.

Deployment steps are in #149: #648218-149: Make API changes in Drupal core be nodes

webchick’s picture

Issue tags: +needs drupal.org deployment

x

drumm’s picture

Committed to drupalorg. I did fix the indentation in drupalorg_crosssite_views_alter, and moved it over to drupalorg_project. Drupalorg crosssite runs on all the sites, and we don't need it everywhere else.

drumm’s picture

Status: Reviewed & tested by the community » Fixed
Issue tags: -needs drupal.org deployment

Deployed.

I recommend getting a fresh development environment after a few of these exist on the live site for any followup work. Request those via the infrastructure queue, or find webchick or me in IRC.

jhodgdon’s picture

drumm: THANKS!!

I've created two change nodes:
http://drupal.org/node/1217104
http://drupal.org/node/1217110

And you can see them listed at
http://drupal.org/list-changes/drupal
and the coder/doc/examples admin view:
http://drupal.org/list-change-updates/drupal

Glitches:

a) Block when there are no related changes... For instance, on this issue, check out the right sidebar. On my test site, I believe I had the view set up so that the block did NOT show if there were no results, but this either wasn't working as I think it was, or it didn't make it into the view export in the feature.

Haven't seen anything else yet...

jhodgdon’s picture

I moved the follow-up above to a separate issue, and I suggest we deal with follow-ups there instead of here:
#1217118: [meta] Followups for API change nodes

Status: Fixed » Closed (fixed)

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

fgm’s picture

Status: Closed (fixed) » Needs work
pillarsdotnet’s picture

Status: Needs work » Closed (fixed)

@#164 by fgm on October 15, 2011 at 7:45am:

Search by keyword ... does not work.

You are correct. Searching by anything else works, but searching by keyword fails.

Please open a new issue to report the problem with the search form, and post a link to it here.