This patch allows extracting of the release notes for the current release of a project.

I have been wanting this for ages so I can easily append the release notes for a contrib upgrade to the commit in client systems.

Comments

moshe weitzman’s picture

I would think that most people want to read the release notes *before* they enable the project. i wonder if we should add a --docs option to dl and updatecode commands which shows you the release notes and README? Just a thought.

greg.1.anderson’s picture

Status: Needs review » Needs work

I think that the usage of this command should be as done in #0 (drush pm-release-notes), but the implementation should be as in drush pm-info -- that is, drush pm-release-notes content to see the release notes for the recommended release, or drush pm-release-notes content-6.x-2.5 to see the release notes for an older version.

luchochs’s picture

Component: Code » PM (dl, en, up ...)
Assigned: gordon » luchochs

This looks promising.
I will try to combine the exposed ideas here and in this related issue: #942738: Display release notes on dl, updatecode, pm-releases.

luchochs’s picture

Status: Needs work » Needs review

[Warning: grammatical error-prone zone.]

Description of proposal:
Prints the release notes to the recommended version by default.
The Drupal major version always refers to that used in the instance.
Command alias = rln.

  1. drush pm-releasenotes [project-name] [--installed-version]
  2. Prints the release notes of the specified project(s). If none is specified, print the release notes for Drupal project.
    If the option --installed-version is set, then the release notes refer to the currently installed version.
    Is possible print the release notes for an specific version, i.e., specifying token-1.13, it prints the notes of the 1.13 version of the Token project corresponding to the Drupal major version used on the site.

  3. drush pm-download [project-name | project-names] --notes
  4. If the --notes option is set, prints the release notes after the project(s) is(are) downloaded.

  5. drush pm-update project-name
  6. Prints the release notes for the specified project before the update is confirmed (responding to the current question: "Do you really want to continue?").

  7. drush pm-update [project-names]
  8. If the specified projects are more of one, or none is specified and need to update more of one, then is necessary to answer (YES or NOT) to the following question:
    "Do you want to see the release notes of the above exposed versions?"
    If YES => Prints the release notes for each project and continues with the update process.
    If NO => The action is omitted and continues with the update process.

  9. drush pm-updatecode project-name
  10. Idem 3.

  11. drush pm-updatecode [project-names]
  12. Idem 4.

Some examples:
$ drush rln cck
$ drush rln cck --installed-version
$ drush rln pathauto zen
$ drush rln token-1.13

$ drush dl token --notes
$ drush dl token-1.10 --notes
$ drush dl pathauto zen --notes

$ dr up project_one project_two
  Code updates will be made to the following projects:
  Project one [project_one-6.x-1.5], Project two [project_two-6.x-1.15]
  Do you want to see the release notes of the above exposed versions? y
  <release notes here>
  Do you really want to continue with the update process?
$ dr up project-one
  <release notes here>
  Do you really want to continue with the update process?

-----

Comments?

moshe weitzman’s picture

This looks really good.

I went to implement this a while back and got a bit annoyed that the Update XML gives us no great way to get the release note HTML. Here is an example XML file - http://updates.drupal.org/release-history/cck/6.x. From there, you can get the release node and from there you have to XPath the release notes themselves and I did not see a clear way to do that. My XPath skills are weak though. We might want to submit a markup patch to project_release module.

greg.1.anderson’s picture

I recommend against changing the markup of the project_release module; there are a lot of existing nodes, and there might be other code that's parsing it. It looks like everything we need is inside

. After you grab .project-release-info via xpath, it might be just as easy to strip out the stuff you don't want with string manipulations.

Don't try to select just the appropriate children of .project-release-info with xpath; not everything is wrapped in paragraph tags. For example, in the case of drush 3.0, there's a bunch of info inside an unordered list.

@luchochs: I'll help refine the xpath if you have any trouble with it. Your proposal looks good.

gordon’s picture

What would be best if there was a xml-rpc call on d.o which would allow you to get a xml version of the node object. That would be the simplest and most generic.

Failing that if we could get an xml-rpc call to return the release information from d.o

moshe weitzman’s picture

@gordon - if available, that would be controlled to just trusted parties. There is unfiltered HTML in there, and probably data that we do not publically disclose. Pursuing XMLRPC with drupal.org is sadly a year long effort IMO.

luchochs’s picture

StatusFileSize
new7.34 KB

This isn't perfect, but it is a beginning; or a continuation rather.

luchochs’s picture

Assigned: luchochs » Unassigned
luchochs’s picture

And...

shell> drush up  token views
... 
Token                           6.x-1.13           6.x-1.15          Actualización disponible 
Views                           6.x-2.10           6.x-2.11          SECURITY UPDATE available                     
...

Code updates will be made to the following projects:
Token [token-6.x-1.15], Views [views-6.x-2.11]
Do you want to see the release notes of the above exposed versions? (y/n): y
------------------------------------------------------------------------------
 > Release notes for Token project [token-6.x-1.15]
------------------------------------------------------------------------------
Changes since DRUPAL-6--1-14:

 * #881492 by Dave Reid: Fixed expand arrows in token tree smooshed too far
   to the left and no longer visible.
 * #883804 by freelock, jpstuddard, Dave Reid: Fixed
   token_get_invalid_tokens_by_context() does not like Location module's
   wildcard tokens.
 * by Dave Reid: Renamed [account-edit] to [account-edit-url] and
   re-organized user tokens.
 * #821008 by Dave Reid: Add token devel pages to inspect tokens on nodes and
   users.
 * #920032 by grobot, Dave Reid: Fixed documentation parameters for
   hook_token_list().
 * #921988 by Dave Reid: Moved duplicate token check to token_requirements().
 * bug #881270 by davereid | greggles: [bookpath], [menupath], [*path] tokens
   not cleaned: aliases without punctuation removed, lower casing, etc.
 * by Dave Reid: Added _token_core_supported_modules().
   by Dave Reid: Fixed documentation and code cleanups.
   by Dave Reid: Fixed theme_token_tree() not listing global token types when
   desired.
 * #927028 by jcmarco, Dave Reid: Fixed token_find_duplicate_tokens() does
   not skip modules that do not return an array.


------------------------------------------------------------------------------
 > Release notes for Views project [views-6.x-2.11]
------------------------------------------------------------------------------
 * #770006 by macdee: Taxonomy term default plugin mysteriously broken.
 * #767146 by bangpound: Validation not passed down into row style plugin.
 * #513396 by yhager and jcisio: Views trim was not multibyte safe.
 * #550420 by blauerberg: Views RSS did not properly pass readmore flag from
   node to RSS item.
 * #781296 by dereine: Node: Type "link to its node" option broken.
 * #769010 by andrea.gariboldi: Overuse of query substitutions fails on some
   databases.
 * #607418: Fix queries broken with "ambiguous nid" due to uncommon
   relationships.
 * #768162: Revert inadvertant change to grid style behavior.
 * #815392 by killes and dereine: Incorrect construction of $account caused
   accidental non-permanent change to $user->roles.
 * #723454 by dereine: Upgrade from 5.0 could cause blocks for views with
   long names to lose configuration.
 * SA-CONTRIB-2010-067: Fix CSRF and XSS vulnerabilities.



Note: Updated projects can potentially break your site. It is NOT recommended to update production sites without prior testing.
Note: A backup of your package will be stored to backups directory if it is not managed by a supported version control system.
Note: If you have made any modifications to any file that belongs to one of these projects, you will have to migrate those modifications after updating.
Do you really want to continue with the update process? (y/n): n

shell> drush dl date-2.5 --notes
Project date (6.x-2.5) downloaded to /home/lucho/proyectos/www/drupal/casadebrujas/sites/all/modules/date.                        [success]
------------------------------------------------------------------------------
 > Release notes for Date project [date-6.x-2.5]
------------------------------------------------------------------------------
Version 6.2.5
=============

NOTE: The included copy of the jQuery UI datepicker in the Date Popups module  
has been removed. It is now necessary to install the jQuery UI module  
(http://drupal.org/project/jquery_ui) for the Date Popups datepicker to work!

- #436490 Add support for the jQuery UI 1.7 datepicker.
- #802046 by t3hk0d3, fix date parsing to better handle unicode characters.
- #475926 by hawleyal, clean html out of ical export.
- #675352 by sdague, clean up ical escape text.
- #848644 by Crell, make sure ical_parse_rrule can handle embedded timezones.
- #784652 by keinstein, fix 'r' format.
- #679224 by brianV, add defaults for month and year for latest jQuery UI  
...

shell> drush rln date
The date project was not found. Is it enabled?                                                                                    [error]
shell> drush en date
The following projects will be enabled: date_timezone, date_api, date
Do you really want to continue? (y/n): y
date was enabled successfully.                                                                                                    [ok]
date_api was enabled successfully.                                                                                                [ok]
date_timezone was enabled successfully.                                                                                           [ok]

shell> drush rln date
Refreshing update status information ...
Done.
------------------------------------------------------------------------------
 > Release notes for Date project [date-6.x-2.6]
------------------------------------------------------------------------------
Version 6.2.6
=============

- Revert #802046. Unicode support change for date parsing causes serious  
errors.

IMPORTANT NOTE: The included copy of the jQuery UI datepicker in the Date  
Popups module has been removed. It is now necessary to install the jQuery UI  
module (http://drupal.org/project/jquery_ui) for the Date Popups datepicker  
to work!

shell> drush upc date
Update information last refreshed: Mar, 11/02/2010 - 11:24

Update status information on all installed and enabled Drupal projects:
 Nombre                          Installed version  Proposed version  Estado                          
...
...
...           


Code updates will be made to the following projects:
Date [date-6.x-2.6]
------------------------------------------------------------------------------
 > Release notes for Date project [date-6.x-2.6]
------------------------------------------------------------------------------
Version 6.2.6
=============

- Revert #802046. Unicode support change for date parsing causes serious  
errors.

IMPORTANT NOTE: The included copy of the jQuery UI datepicker in the Date  
Popups module has been removed. It is now necessary to install the jQuery UI  
module (http://drupal.org/project/jquery_ui) for the Date Popups datepicker  
to work!



Note: Updated projects can potentially break your site. It is NOT recommended to update production sites without prior testing.
Note: A backup of your package will be stored to backups directory if it is not managed by a supported version control system.
Note: If you have made any modifications to any file that belongs to one of these projects, you will have to migrate those modifications after updating.
Do you really want to continue with the update process? (y/n): y
Project date was updated successfully. Installed version is now 6.x-2.6.
Backups were saved into the directory /home/lucho/.drush-backups/20101102143647/modules/date.                                     [ok]
'all' cache was cleared                                                                                                           [success]

moshe weitzman’s picture

Nice!

In your example, you are upgrading ahead two releases for token module. Should we show notes for both releases? That could get verbose in the shell but I suspect that users want to see it all since they specifically requested --notes.

greg.1.anderson’s picture

Status: Needs review » Needs work

Wow, well done! I didn't know about drupal_html_to_text; that's a good solution.

I agree with Moshe about showing multiple releases during update. The potential for very long output brings up the following usage scenario.

User runs pm-update and sees tons of output and bails with "no". User re-runs pm-update, this time piping the output through "more". This works fine until the yes/no prompt shows up, at which point output hangs due to the redirection. Since the yes/no prompt happens before the release notes, the user bails out again with control-c and runs pm-releasenotes.

The main conclusion I draw from the above is that pm-releasenotes, when run with no parameters, should show all of the release notes for all versions newer than what is installed. If there are no releases newer than the one installed, then it should display the release notes for the current release. There should be some text at the head of the output that says which is being displayed, the newer release or the current release, and there should be a flag (--current?) to make it work like drush dl xyzzy --notes (which works exactly as it should).

The second conclusion I draw from above is that it sure would be nice to run pm-releasenotes through a pager when called from pm-updatecode. We could just pipe through more on Linux via a system call to sh -c drush pm-releasenotes ... | more, but that would not be portable. Ideally we would build our own pager, I guess. :( I see the issue of pagination as optional.

luchochs’s picture

Regarding drupal_html_to_text, is really appropriate; gordon's merit. The only disadvantage that has is that it does not exist in D5.
I agree with the suggestions, I'll see what I can do about it. Of course any contribution is welcome.

luchochs’s picture

Assigned: Unassigned » luchochs

The new version of pm-releasenotes is underway. News soon (good news, I hope).

luchochs’s picture

Some alternatives for the header:

------------------------------------------------------------------------------
-------- RELEASE NOTES FOR TOKEN PROJECT, VERSION 6.X-1.13 -------------------

 > Last updated: June 7, 2010 - 15:00.
 > Note: this is the current version.
------------------------------------------------------------------------------
 * by Zoltan Balogh: Updated Hungarian translation
 * feature #511026 by greggles: provide token for number of comments on a
   node
 * feature #520154 by greggles: Provide number of new comments token
...


------------------------------------------------------------------------------
 > Release notes for Token project, version 6.x-1.14:
 > Last updated: August 11, 2010 - 18:01.
 > Note: the current version is 6.x-1.13.
------------------------------------------------------------------------------
 * #252241 by EmanueleQuinto, isaac.niebeling, Dave Reid: Added [log] and
   [log-raw] tokens for node revision log message.
 * #798684 by Dave Reid, neilnz: Added [menu-link-mlid] and [menu-link-plid]
   tokens.
 * by Dave Reid: Backported token_get_invalid_tokens_by_context().
...


-------- RELEASE NOTES FOR TOKEN PROJECT, VERSION 6.X-1.15 -------------------

 > Last updated: October 7, 2010 - 16:46.
 > Note: this is the recommended version
------------------------------------------------------------------------------
 * #881492 by Dave Reid: Fixed expand arrows in token tree smooshed too far
   to the left and no longer visible.
 * #883804 by freelock, jpstuddard, Dave Reid: Fixed
   token_get_invalid_tokens_by_context() does not like Location module's
   wildcard tokens.
 * by Dave Reid: Renamed [account-edit] to [account-edit-url] and
   re-organized user tokens.
...

The blank line after the title is a feature that has the function drupal_html_to_text() when using header tags (<h2> in these cases).

I prefer the option 6.x-1.14. Some other opinions/alternatives?

greg.1.anderson’s picture

I like the alternative you recommend too, but could be perfectly happy with any of the variants shown above, myself.

luchochs’s picture

Assigned: luchochs » Unassigned
Status: Needs work » Needs review
StatusFileSize
new19.47 KB
new67.44 KB

A brief description of the changes in this patch:

Both rln and up/upc commands show release notes for current version, recommended version and for all the versions between these two.
A status report is exhibited in the head:
> Note: this is the current & recommended version.
> Note: this is the current version.
> Note: the current version is $version.
> Note: this is the recommended version.
This reference is shown in the rln and up/upc commands but not on the dl.
The option --installed-version is no longer necessary.
Also a line in the header refers to the last time that the notes were updated, e.g:
> Last updated: August 11, 2010 - 18:01.

The .txt file shows how it works on a (very) outdated test-site.

greg.1.anderson’s picture

Status: Needs review » Needs work

This is super-awesome -- a major enhancement to drush.

Some notes.

drush dl eightball --notes gives a strange error if there is no bootstrapped site. dl does not require a bootstrapped site, so --notes should check and print a nice explanation if one is not available.

I wonder if the 'dl' command should ask "Do you want to display the release notes?" after the download is completed (feature advertising).

When rln is showing multiple releases, it says "this is the current release". I think it should say "Installed", like pm-releases does.

Overall, this works wonderfully. Amazing work!

moshe weitzman’s picture

Yeah, very nice. I just looked at the sample output and have some comments:

$ dr rln zen
The zen project was not found. If it is enabled try again after      [error]
running pm-refresh command.
$ dr en zen
The following projects will be enabled: zen
Do you really want to continue? (y/n): y
zen was enabled successfully.                                        [ok]
$ dr rln zen
The zen project was not found. If it is enabled try again after      [error]
running pm-refresh command.
$ dr pm-refresh
Refreshing update status information ...
Done.

This sequence is not too happy. The major problem is that rln should work on any random project, not just enabled modules/themes. It should be ideally be like dl command and not rely even on a working drupal. I should be able to do rln zen and get latest release notes. rln zen-1.3 should get the specified notes regardless of whether I have zen or not.

moshe weitzman’s picture

I think pm-releases should also work as I have described.

luchochs’s picture

Greg,
1. Strange error? More info is welcome.
2. I agree with the proposal.
3. Good detail.

Moshe,
Know the realease notes for a project that I'm not using makes little sense from my point of view, hence the design; but if the needs are others it can be adapted, of course.
Regarding pm-releases, it does.

moshe weitzman’s picture

I do think there is a solid use case. Consider that I have used pathauto module in the past but haven't followed it in a while. Would be great to see whats been happenning in last few releases. That would influence my decision to pm-download it. I'll agree that this is not the primary use case but I think it is worth supporting.

luchochs’s picture

Title: Add pm-release-notes command to extract the release notes from drupal.org » Add pm-releasenotes command to extract the release notes from drupal.org
Assigned: Unassigned » luchochs

With regard to Greg's suggestion in #19-2:
On second thought, the user is already saying he wants to see the notes using the option --notes. The message of confirmation does not seem to me to be necessary.

greg.1.anderson’s picture

Regarding the strange error:

Unable to include the update_info engine drupal from /home/ga/local/drush/commands/pm/update_info.

Fatal error: Call to undefined function _pm_get_update_info() in /home/ga/local/drush/commands/pm/pm.drush.inc on line 1002

It's not mysterious, just strange. This could be avoided with a call to function_exists(), but moshe's request to re-code pm-releasenotes and pm-releases to not require the update_info engine (like pm-download) would also make this go away.

My suggestion for #19-2 was for instances where the user ran pm-download -without- the --notes option. If --notes was specified, then the notes should be printed without a confirmation message. Perhaps --no-notes could skip the confirm and not print the notes.

luchochs’s picture

Thanks for the additional data.

The value of the update_info engine for pm-releases is pm_get_project_info(). My idea is to use as helper the drush_pm_releases() function to gain independence and it does not matter if the module is enabled.
Any known problems using the pm-releases command?

Regarding pm-download and --notes, not sure if show a confirmation message by default is appropriate.

luchochs’s picture

Assigned: luchochs » Unassigned
Status: Needs work » Needs review
StatusFileSize
new15.07 KB

New approach. Now also is possible to see the release notes for a project though it is not installed.

luchochs’s picture

StatusFileSize
new15.08 KB

A couple of changes:
- drush_set_error() instead of drush_log(), and
- search for installed packages even if a version was specified.

luchochs’s picture

Clarification of what I mean by "packages":
Package = group of modules and/or themes in a project.
The command pm-releases have no way of knowing, for example, we installed version of CCK. This is because the array of projects generated by drush_get_projects() function (and used by pm-releases to determine whether a project is installed) lists modules and themes, not projects that contain them, i.e., it does not list packages.

moshe weitzman’s picture

Assigned: Unassigned » jonhattan

Would be great to get reviews from jonhattan and greg and anyone else who is interested.

greg.1.anderson’s picture

I looked at this briefly and was puzzled by $set_args and $get_args. It seems like this might not be correct, or at least should simplify to something more readable, but I'm going to have to look at it more closely and try a couple of tests when I have more time.

Or maybe luchochs could just re-roll so that drush_pm_releases() called _drush_pm_releases(func_get_args(), TRUE); (or whatever flags are necessary...) That wouldn't befuddle my brain so much.

luchochs’s picture

Greg, briefly:
$get_args: a Boolean value. If TRUE, the function takes the arguments that the user proposes. If FALSE, the function takes as arguments those given by another function (i.e., $set_args; probably another name is more appropriate). If TRUE and the user not specify any argument we have a special case, also considered.

luchochs’s picture

StatusFileSize
new14.91 KB

Re-rolling, now $set_args is $projects.

luchochs’s picture

StatusFileSize
new16.42 KB

Re-re-rolling. Sorry.

luchochs’s picture

StatusFileSize
new15.84 KB

Hmmmm. :)

luchochs’s picture

StatusFileSize
new15.81 KB

Sunday, error-prone Sunday.
Last re-roll for today. Removed code that is not related to this issue and added a check for possible mismanagement of versions (as was done in the previous approach, #18).
My imperfect English is well known in these parts, and probably the code comments are not the exception. My apologies about it.

greg.1.anderson’s picture

Well, it wasn't the name of the variable that was befuddling me. It's just that $get_args can in some instances be the second element in the 'projects' array. Just to be sure that I wasn't crazy, I added a print statement:

% drush @dev pm-releases views cck
get_args is cck and projects is views,cck

Even though $get_args is "cck" instead of TRUE or FALSE, the code still works. I can be deliberately abusive, though:

$ drush @dev pm-releases views 0
WD php: implode(): Invalid arguments passed in
/home/ga/local/drush/commands/pm/pm.drush.inc on line 917.
get_args is 0 and projects is 
WD php: Invalid argument supplied for foreach() in
/srv/www/home.greenknowe.org/includes/common.inc on line 1651.
implode(): Invalid arguments passed in
/home/ga/local/drush/commands/pm/pm.drush.inc on line 917.
Invalid argument supplied for foreach() in
/srv/www/home.greenknowe.org/includes/common.inc on line 1651.

Yes, I know, I am a big meanie for passing in "0" for the second argument, which forces get_args to FALSE and messes up the logic of the code. I think that you should fix this using the technique I suggested in #31, not so much because anyone is likely to submit a project named "0" , but simply to make the code more clear by avoiding the befuddling override of $projects[1].

drush_shell_exec does something similar to what I suggest.

Beyond this one fairly trivial issue, the code works as intended, and looks good once the issue of where $projects comes from is worked out.

luchochs’s picture

I see, good point; thanks for the review!

I think that you should fix this using the technique I suggested in #31

Totally agree. Soon new patch with this technique also applied to drush_pm_releasenotes().

luchochs’s picture

StatusFileSize
new16.4 KB

Changes:
- Implementation of the technique suggested in #31/#37.
- Now _drush_pm_releases() is called only once per _drush_pm_releasenotes(), independently of the quantity of requested projects.
- Most appropriate log messages.

jonhattan’s picture

reviewing.

jonhattan’s picture

Status: Needs review » Needs work

a) I think pm-updatecode should not show release notes for installed version since we are moving to a new version.
b) The sentence:RELEASE NOTES FOR 'CONTENT CONSTRUCTION KIT (CCK)' PROJECT, VERSION 6.x-2.8: can be shorten, at least to avoid using two lines.

On the code: there're some comments not directly related to your patch but with a need to be fixed and may be addressed altogether:

  1. `drush dl --notes --pipe`. Just to prevent: if --pipe it shouldn't print the notes.
  2. _drush_pm_releases() have tangled functionality. It can be a cleaner , helper function if it just returns that $merged_info (by adding our own data to $info and get rid of $specific_data()) and moving out table generation ($rows, $status strings) to drush_pm_releases(). This way we also get rid of $helper argument.
  3. use $info instead of $projects_info in _drush_pm_releasenotes() to follow terminology.
  4. _pm_releases should also do project name validation. It is missing for pm-releases and you're doing it already in rln:
        $project_info = $projects_info[$project];
        if (is_null($project_info)) {
          drush_set_error('DRUSH_PM_PROJECT_NOT_FOUND', dt("The !project project was not found.", array('!project' => $project)));
          continue;
        }
    

    . Note the correct thing is if (!is_set($projects_info[$project])). To avoid warnings in E_STRICT.

I'd like to see at least 2 and 4 fixed before committing.I'll also fix myself minor things including some code comments before the commit.

@luchochns if you're not willing to work on 2 and 4 I'll take over.

luchochs’s picture

First of all, thanks for the review.

Regarding the suggestions:
a - Makes sense to me. It should be noted that sometimes the release notes refer to changes made in the previous version, but I think they are exceptional cases.
b - Few projects should have a title as long as the mentioned one, but if needed to change the current format other alternatives are welcome.

Later I'll see what I can do about the four needs.

luchochs’s picture

Tentative new format for pm-releases:

$ drush pm-releases token cck
 Project   Release      Date         Status                 
 -------   --------     -----------  ----------             
 token     6.x-1.15     2010-Oct-07  Supported, Recommended 
 token     6.x-1.14     2010-Aug-11                         
 token     6.x-1.13     2010-Jun-07  Installed              
 token     6.x-1.12     2009-Jun-01                         
 token     6.x-1.11     2008-Aug-02                         
 token     6.x-1.10     2008-Feb-23                         
 token     6.x-1.x-dev  2010-Nov-17  Development            

 Project   Release        Date         Status                           
 -------   --------       -----------  ----------                       
 cck       6.x-3.x-dev    2010-Nov-10  Supported, Development           
 cck       6.x-2.8        2010-Aug-11  Supported, Recommended, Security 
 cck       6.x-2.7        2010-Jun-16  Security                         
 cck       6.x-2.6        2009-Nov-05                                   
 cck       6.x-2.5        2009-Aug-03                                   
 cck       6.x-2.4        2009-Jun-17                                   
 cck       6.x-2.3        2009-Jun-04                                   
 cck       6.x-2.2        2009-Mar-18  Security                         
 cck       6.x-2.1        2008-Nov-11                                   
 cck       6.x-2.0        2008-Nov-05  Security                         
 cck       6.x-2.0-rc10   2008-Oct-08                                   
 cck       6.x-2.0-rc9    2008-Oct-06                                   
 cck       6.x-2.0-rc8    2008-Oct-02                                   
 cck       6.x-2.0-rc7    2008-Sep-09                                   
 cck       6.x-2.0-rc6    2008-Aug-15                                   
 cck       6.x-2.0-rc5    2008-Aug-14                                   
 cck       6.x-2.0-rc4    2008-Jul-10                                   
 cck       6.x-2.0-rc3    2008-Jul-03                                   
 cck       6.x-2.0-rc2    2008-Jul-02                                   
 cck       6.x-2.0-rc1    2008-Jul-02                                   
 cck       6.x-2.0-beta   2008-Jun-03                                   
 cck       6.x-2.x-dev    2010-Nov-10  Development                      
 cck       6.x-1.0-alpha  2008-Apr-24                                   
 cck       6.x-1.x-dev    2010-Jul-10  Development

Jonhattan, item 4 is currently done (we could add the name of the not found projects to the log message if $info is empty, i think).

Update: I added the command to the output pasted.

jonhattan’s picture

I prefer current format for pm-releases. Anyway such a proposal should be done in a separate issue to not get stalled in this one, as we're wanting to have a 4.0 release the day of drupal 7.

btw, did you missed a patch in last comment? Please attach your last work on this in order to review and be close to commit.

luchochs’s picture

Jonhattan, your words:

On the code: there're some comments not directly related to your patch but with a need to be fixed and may be addressed altogether:

A few lines down in the same comment:

I'd like to see at least 2 and 4 fixed before committing.

Then, the proposed format change in #43 related to the code that must be addressed all together. I think it's an improvement, then show them to avoid last minute surprise opinions.

And later:

Anyway such a proposal should be done in a separate issue to not get stalled in this one, as we're wanting to have a 4.0 release the day of drupal 7.

Seems a bit contradictory.

-------------
Regarding the missing patch, don't worry, I will change the issue's status when appropriate.

luchochs’s picture

Assigned: jonhattan » luchochs

Thinking it better, it seems to me better to show also release notes for the installed version.
New patch soon, addressing #41-1, #41-2, #41-3 and #43.

greg.1.anderson’s picture

Per #21, I think it is reasonable to work on both pm-releases and pm-releasenotes in the same issue, especially since, if I am not mistaken, pm-releasenotes calls pm-releases -- so they are tangled. We want both pm-releases and pm-releasenotes for drush-4, ideally. I think we are close to done here...

I like the improved output for pm-releases in #43, but the current output is okay with me too if others feel it should not change.

jonhattan’s picture

In #41 I propose to make some changes to ship better, untangled code. Is fine to discuss #43 here but it is just a cosmetic change to other command output, so in #44 I ask to treat that in a separate issue to facilitate the commit of the big release notes feature. perhaps I was unnecessary haste.

OTOH, #21 is about commands working in despite a project is downloaded/enabled.

Ok, lets relax. We can discuss #43 here.

Current format:

$ drush pm-releases zen tao
 Project  Release        Date         Status                 
 zen      6.x-2.0        2010-Jun-26  Supported, Recommended 
 zen      6.x-2.0-beta1  2009-Nov-10                         
 zen      6.x-2.x-dev    2010-Nov-12  Development            
 zen      6.x-1.1        2009-Nov-10  Supported              
 zen      6.x-1.0        2009-Feb-13                         
 zen      6.x-1.0-beta3  2008-Sep-15                         
 zen      6.x-1.0-beta2  2008-May-20                         
 zen      6.x-1.0-beta1  2008-May-14                         
 zen      6.x-1.x-dev    2010-Nov-12  Development            
 tao      6.x-3.2        2010-Oct-21  Supported, Recommended 
 tao      6.x-3.1        2010-Sep-28                         
 tao      6.x-3.0        2010-Sep-28        

Proposal in #43 just split this in two tables and the first column becomes more redundant. I think an output with a header like any proposal in #16 is more appropiate. For example:

---- RELEASES FOR ZEN PROJECT ------
 Release        Date         Status                 
 6.x-2.0        2010-Jun-26  Supported, Recommended  
 6.x-2.0-beta1  2009-Nov-10                         
[...]

---- RELEASES FOR TAO PROJECT ------
 Release        Date         Status                 
 6.x-3.2        2010-Oct-21  Supported, Recommended 
 6.x-3.1        2010-Sep-28                         
 6.x-3.0        2010-Sep-28        
greg.1.anderson’s picture

Consistency between pm-releasenotes and pm-releases per #48 seems like a good thing to me.

luchochs’s picture

Yes, good suggestion.
It seems we'll have to set common width for the first column via drush_print_table()'s third parameter.

Regarding #41-b, a solution might be to use the short name of the project instead of the title, i.e. 'CCK' instead of 'CONTENT CONSTRUCTION KIT (CCK)'.

luchochs’s picture

Assigned: luchochs » Unassigned
Status: Needs work » Needs review
StatusFileSize
new16.91 KB

Well, my participation in this issue ends here.
Thank you Gordon for the seed; thank you Greg, Jonhattan and Moshe for the ideas, suggestions and reviews.

Changes:
- #41-b (#50).
- #41-1, #41-2 & #41-3.
- #43 (last line).
- #48.

jonhattan’s picture

Assigned: Unassigned » jonhattan
jonhattan’s picture

Status: Needs review » Fixed

Commited (http://drupal.org/cvs?commit=453712) with several changes to rearrange code (#41-2,3,4), also print releasenotes for updatecode of drupal core and also added more //comments.

There're some improvements/changes I think of. Will post a follow up issue.

jonhattan’s picture

Continuation: #977584: Improvements to pm-releasenotes and other commands leveraging it..

@luchochs: can you explain what does this piece do? I don't understand the message.

1162 	  	       // ?
1163 	  	       if ($project['recommended'] != $project['minor_supported']) {
1164 	  	         $status_msg = dt("Note: Unwanted versions management. Run 'drush pm-releases !project' for more information.", array('!project' => $key));
1165 	  	       }

(Note in your patch it was split in two functions and $project['warning'] was used).

luchochs’s picture

StatusFileSize
new19.96 KB

Jonhattan, yes, this piece of code effects a check for possible mismanagement of versions. See #36. An example can be seen in the attached .txt file in #18, related to the eightball project.

Committed patch (#53) attached for reference.

luchochs’s picture

Status: Fixed » Needs review

My first impression of the changes made in #55 is not very good. My opinion when I have more time.
P.S.: Jonhnattan, you test #55 before commit it, right?

greg.1.anderson’s picture

Status: Needs review » Fixed

Congratulations on getting this committed. I ran the checked-in code through some light testing, and I must say I'm really thrilled with it.

Let's continue discussing improvements in issue #977584: Improvements to pm-releasenotes and other commands leveraging it. (or in separate issues as appropriate).

luchochs’s picture

Status: Fixed » Needs work

Thanks, but i think that some things need to be corrected here:
- No show notes in header's rln.
- If the user add a specific version, rln not works as it should.
- Error reports could be improved.

A basic rule that seems to me appropriate to add in order that to work in group makes sense: If you modify something, SHOW IT so someone else can review the changes, and then commit.

greg.1.anderson’s picture

You're right, versions do seem to be a bit funky in the committed code.

I thought it would be a good time to split off to a new issue for improvements, but if you'd rather continue to post bug fixes and other patches here, I'm happy to keep reviewing them. Thanks for all of your contributions.

jonhattan’s picture

Status: Needs work » Fixed
StatusFileSize
new3.77 KB

I did introduce a last minute change after refactoring the code. Now fixed in cvs along with:

1. detect unpublised projects (they have no releases).
2. deactivate --notes in drush dl if no full bootstrap or update.module not enabled (#19).
3. add a comment to document the case for eightball.

patch attached for your convenience.

---
The annoying result of pm-releases has an explanation:

------- RELEASES FOR 'EIGHTBALL' PROJECT -------
 Release         Date         Status                 
 6.x-1.2-beta2   2009-Apr-24  Supported              
 6.x-1.2         2009-Apr-23  Recommended, Installed 
 6.x-1.1         2009-Feb-03             

supported versions are major version numbers, not major-minor. All of those releases are supported but it happen that we add the 'supported' flag to the first release found (latest published), to not be repetitive.
Beyond that, we are in concordance with releases in the project's page: dl, pm-releases, rln by default will select 6.x-1.2 as the recommended version. The exception is pm-udpatecode. with recent Greg's patch in #760814: drush pm-update offers to update to an older release than currently installed, and misses the newest available release latest is selected, that in eightball's case seems desirable.

Anyway I don't think we should consider this and other possible edge case that ocurrs remotely: it is a bad management of releases by the project maintainer*, probably are other ways to mess releases we are not addressing, neither we can't anticipate to.

* He wanted to create a -dev release and ended up creating that confusing beta2 with a recent date than the stable-recommended one --and he didn't correct it for more than a year.

luchochs’s picture

Yes, agree with the above on the mismanagement of versions.
I called this 'minor_supported' in ($project['recommended'] != $project['minor_supported']) because some projects are more than one major version supported and it always refers to the smallest value, but ... reviewing this again, I did my analysis wrong using as parameters only 'views' and 'cck' where the "max_supported" (to call it somehow) are -dev and -alpha releases, i.e., not recommended; and this isn't always this way, such as happens with 'zen'.
In the previous pm-releasenotes' approach (#18) I used a different (and effective) method that verify if there exists 'extra_version' only in the recommended version. I do not know if you want to go back to using this method or not more test about it.

Thanks for the patch (and sorry for my English), later I will make some test.

luchochs’s picture

Clarification: verify if there exists 'extra_version' only in the recommended version, being the checked versions the installed one and the recommended one.

luchochs’s picture

Status: Fixed » Needs review
luchochs’s picture

Yes, thanks for the patch ...
Apparently I missed a small detail in your last comment Jonhattan:

Now fixed in cvs

I do not understand your position.

My time has nothing to do here.

greg.1.anderson’s picture

Status: Needs review » Fixed

Side note related to comment in #60: My change in pm-updatecode was intended to only take the latest release when the installed version is newer than the recommended version. I think it does so, but please open another issue if it does not.

The commit from #60 is working well in my tests.

luchochs’s picture

I still do not understand why this code is already committed, if someone can explain it to me, my brain thanks you.

jonhattan’s picture

Status: Fixed » Active

@luchochs I decided to commit at #53 because the patch is very big and it was melting my brain to follow up any new change (it took me 5 hours --yes, five-- to understand, cleanup and rework #51 to #53).

By committing we have a new revision to work patches on. Newer patches are smaller and easier to track. Anyone can continue working over the commited code. In fact in #60's patch it is easy to see the fix for that last minute error. I also shipped in that commit some fail checks and decided to directly commit as those fixes seems evident to me.

Even more first commit was intented to be quite stable. It was think to allow users to start testing ASAP --in order to enter a release cicle soon.
Of course I did test it to death before committing, but I introduced a basic error: wrong variable name in two places. Second commit just fixed it quick, to continue offering testers a stable implementation of pm-releasenotes.

I also think issues with a lot of comments are hard to follow: look us referencing back to previous comments and sections-in-comments and some implementation decisions in attachment to comments.

^^those are my reasons.

Your time, and your contributions are very valuable --for what I thank you-- so please don't feel you has nothing to do here.

On english (engrish is used as a joke).... I'm neither english native speaker. If you use IRC we can meet and speak spanish if you feel more comfortable. I'm hooked to #drupal, #drupal-es ,..

I'll keep this issue open to finish discussion on the mismanagement of versions, or if any bug on recently commited code is found. But I'd like to work new funcionalities in #977584: Improvements to pm-releasenotes and other commands leveraging it..

Mismanagement of versions:: I think it is all summarized in #60 and #61. As I already said, I'm for removing the special case for eightball module. Now my mind is dettached from that and I'm not in context to understand what you say en #61. But can come later and dive into it. Now it's time for paid work.

luchochs’s picture

Jonhattan, now only tell you I appreciate the explanation. I'll add my reply a little later today.
Update: 'reply' instead of 'answer'.

luchochs’s picture

Status: Active » Needs work

#68's complement:

What I had expected post-#51: If the patch attached in #51 needed more work, attach the new version and proceed with the work-review cycle until those who are interested in giving their opinion feel satisfied with the result. I would like to review your patch before you commit it (and probably someone else too), I think that is well expressed in my recent comments.
Later I saw a patch on your new comment. "Good! Let's solve all known bugs here, so then commit all together and then continue with the improvements they're needed elsewhere" - I thought. But not, this thread became too prone to commit without the prior opinion of someone other than the patch's creator. Then my discontent grew a little more and that was expressed through the engrish-poetic phrase "My time has nothing to do here".

With regard to the language, is a major limitation for me, but are the rules here, so I try to do the best I can about it, believe me. The IRC is a good option, I'm not hooked at the moment but if this changes I will send a ping.

I consider my time and work just as valid as anyone else who has intentions to provide constructive feedback (via code or words), my thanks were directed toward them in #51.

Change the issue's status to "need work" so we can see if there is an effective way to verify possible mismanagement of projects even if they are not installed, or if it is worth applying the previous method described in #61 and #62, or no more "warning message". Something must be done about it (this is a bug: $project['recommended'] != $project['minor_supported'] ).

P.S.: If any of my action/thought is not appropriate, then anyone let me know please.

greg.1.anderson’s picture

Status: Needs work » Active

@luchochs: I really appreciate your contribution and hard work on this issue. The feature is a great addition to drush, and while it may need some more work and repair, I'm very happy to have it in drush.

In #51, when you said "Well, my participation in this issue ends here," I interpreted that to mean that you were no longer going to submit or review patches on this feature. Given that comment, I think that it was appropriate for jonhattan to commit the last patch with his corrections. Of course I am very happy that you are still participating in this issue, because your contributions here are valued and appreciated.

We always try to review code to perfection so that it can be committed without defects, but sometimes bugs are committed for whatever reason. I've done it myself. Usually, once a new feature is committed, we address additional problems and changes in new issues. Things can sometimes get confusing if the review cycle continues past the point where code is committed. For example, usually "active" means there is no patch, "needs review" means there is a patch that someone should look at, and "needs work" means there is a patch, but it is not ready to be committed yet. After a patch is committed, I would recommend continuing the same convention as if the committed code is the new baseline (and really it is), so right now we would be in the "active" state. However, leaving this issue "active" could mess up the release notes for the next release if we don't set this back to "fixed" before drush-4 (hopefully we will...)

I think it would be less confusing if we moved forward and made a new issue for the remaining bugs and enhancements to the pm-releasenotes feature, but I will gladly accept your comments in whatever form you care to submit them. I'm getting a bit confused in #69 because we're building up a lot of back references. If we just forgive and forget the past and move on from the current code, I think we'll get the real issues straightened out quickly.

Thank you again for your time and contributions.

luchochs’s picture

Status: Active » Fixed

Greg, O.K., no more patches here.
A few more words from me:

In #51, when you said "Well, my participation in this issue ends here," I interpreted that to mean that you were no longer going to submit or review patches on this feature.

Yes, you interpreted well my comment. I said it because I was thinking that to the code he needed only minimal retouches.

Given that comment, I think that it was appropriate for jonhattan to commit the last patch with his corrections.

I do not agree here.
I misunderstand the concept that had Jonhattan on make _drush_pm_releases() a clean & helper function. Then the code needed something more than minor tweaks, and Jonhattan did a great job on this.
Then #53. At this point in this thread no longer work as before. In fact, in my view, do not follow basic rules to follow in any work group, either here in the Drupal community as in any work environment.

On the other hand, if my participation in this topic would have ended at #51, I think the opinion of the rest should be taken into account. How? Simply showing the changes and: Status: needs work » needs review.
(I'm being repetitive in some respects, probably.)

We always try to review code to perfection so that it can be committed without defects, but sometimes bugs are committed for whatever reason.

1. Luckily I know no one perfect.
2. I can attest that do an excellent job here. This may be a good opportunity to say:

Thanks guys for making Drush what it is.

Status: Fixed » Closed (fixed)

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