[WIP] Upload a patch to WordPress Trac based on the current directory diff #32

Merged
merged 2 commits into from Apr 2, 2016

Conversation

Projects
None yet
3 participants
@ericandrewlewis
Contributor

ericandrewlewis commented Dec 30, 2015

In reference to #30, this is a work in progress for adding the functionality to upload a patch to a ticket on Trac based on the local WP repository diff.

Feedback welcome! 馃槃

@ericandrewlewis

This comment has been minimized.

Show comment
Hide comment
@ericandrewlewis

ericandrewlewis Jan 18, 2016

Contributor

@aaronjorbin can you review this? I'd consider this an MVP at this point.

For the first version, I'd like to explicitly request username and password when a user uploads a patch. For a V2.0, we can look into utilizing password caching mechanisms in each OS.

Contributor

ericandrewlewis commented Jan 18, 2016

@aaronjorbin can you review this? I'd consider this an MVP at this point.

For the first version, I'd like to explicitly request username and password when a user uploads a patch. For a V2.0, we can look into utilizing password caching mechanisms in each OS.

tasks/patch_wordpress.js
_.str = _.str = require('underscore.string')
_.mixin( _.str.exports() )
+var rl = readline.createInterface({

This comment has been minimized.

@aaronjorbin

aaronjorbin Jan 24, 2016

Member

Elsewhere in the task, inquirer is used for asking questions. What benefit do you think directly using readline has over it?

@aaronjorbin

aaronjorbin Jan 24, 2016

Member

Elsewhere in the task, inquirer is used for asking questions. What benefit do you think directly using readline has over it?

This comment has been minimized.

@ericandrewlewis

ericandrewlewis Jan 24, 2016

Contributor

None 馃槈 I'll integrate with inquirer.

@ericandrewlewis

ericandrewlewis Jan 24, 2016

Contributor

None 馃槈 I'll integrate with inquirer.

This comment has been minimized.

@aaronjorbin

aaronjorbin Jan 24, 2016

Member

馃槏

tasks/patch_wordpress.js
+ grunt.log.debug( 'options: ' + JSON.stringify( options ) )
+
+ var requestTicketNumber = function() {
+ rl.question('Enter the ticket number: ', function(_ticketNumber) {

This comment has been minimized.

@aaronjorbin

aaronjorbin Jan 24, 2016

Member

In the patch_wordpress task, we take the ticket number in as a part of the cli call? What do you think of doing that here to stay consistent?

@aaronjorbin

aaronjorbin Jan 24, 2016

Member

In the patch_wordpress task, we take the ticket number in as a part of the cli call? What do you think of doing that here to stay consistent?

tasks/patch_wordpress.js
@@ -280,4 +285,76 @@ module.exports = function(grunt) {
})
+ grunt.registerTask( 'upload_patch_to_wordpress_trac', 'Upload the current diff of your develop-wordpress directory to WordPress trac', function( ticketNumber ) {

This comment has been minimized.

@aaronjorbin

aaronjorbin Jan 24, 2016

Member

this plugin is used by core WordPress, but also bbpress and buddypress (and for all I know, more places as well, but I doubt it). How about upload_patch_trac as the name? Yes, WordPress is in the name of the project, but it basically means "wordpress style projects" for us.

@aaronjorbin

aaronjorbin Jan 24, 2016

Member

this plugin is used by core WordPress, but also bbpress and buddypress (and for all I know, more places as well, but I doubt it). How about upload_patch_trac as the name? Yes, WordPress is in the name of the project, but it basically means "wordpress style projects" for us.

This comment has been minimized.

@ericandrewlewis

ericandrewlewis Jan 24, 2016

Contributor

That makes sense. How about even upload_patch?

@ericandrewlewis

ericandrewlewis Jan 24, 2016

Contributor

That makes sense. How about even upload_patch?

This comment has been minimized.

@aaronjorbin

aaronjorbin Jan 24, 2016

Member

Sounds great to me.

@aaronjorbin

aaronjorbin Jan 24, 2016

Member

Sounds great to me.

tasks/patch_wordpress.js
+ } else {
+ grunt.log.error( 'Something went wrong: ' )
+ grunt.log.error( err )
+ grunt.log.error( 'Let\'s start over.' )

This comment has been minimized.

@aaronjorbin

aaronjorbin Jan 24, 2016

Member

I like failing and erroring out rather than starting over and forcing someone to quit. That may just be personal preference, I don't know.

@aaronjorbin

aaronjorbin Jan 24, 2016

Member

I like failing and erroring out rather than starting over and forcing someone to quit. That may just be personal preference, I don't know.

This comment has been minimized.

@ericandrewlewis

ericandrewlewis Jan 24, 2016

Contributor

That sounds just as good, I'll refactor.

@ericandrewlewis

ericandrewlewis Jan 24, 2016

Contributor

That sounds just as good, I'll refactor.

@ericandrewlewis

This comment has been minimized.

Show comment
Hide comment
@ericandrewlewis

ericandrewlewis Jan 24, 2016

Contributor

Modified the PR based on feedback, squashing commits.

Contributor

ericandrewlewis commented Jan 24, 2016

Modified the PR based on feedback, squashing commits.

@aaronjorbin

This comment has been minimized.

Show comment
Hide comment
@aaronjorbin

aaronjorbin Jan 24, 2016

Member

@ericandrewlewis Can you rebase and resolve the conflicts so I can properly test?

Member

aaronjorbin commented Jan 24, 2016

@ericandrewlewis Can you rebase and resolve the conflicts so I can properly test?

@ericandrewlewis

This comment has been minimized.

Show comment
Hide comment
@ericandrewlewis

ericandrewlewis Jan 24, 2016

Contributor

Yep ~ rebased!

Contributor

ericandrewlewis commented Jan 24, 2016

Yep ~ rebased!

@ntwb

This comment has been minimized.

Show comment
Hide comment
@ntwb

ntwb Jan 25, 2016

Member

I just tested this pull request for bbPress trac:

The ticket: https://bbpress.trac.wordpress.org/ticket/2907

The terminal commands used and output:

netweb@MacBook ~/dev/bbpress.git.wordpress.org [master]
$ git status
On branch master
Your branch is up-to-date with 'origin/master'.
Changes not staged for commit:
  (use "git add <file>..." to update what will be committed)
  (use "git checkout -- <file>..." to discard changes in working directory)

    modified:   src/templates/default/js/user.js

no changes added to commit (use "git add" and/or "git commit -a")

netweb@MacBook ~/dev/bbpress.git.wordpress.org [master]
$ grunt upload_patch:2907
Running "upload_patch:2907" (upload_patch) task
? Enter your WordPress.org username netweb
? Enter your WordPress.org password ********************************************************
Uploaded patch.

Done, without errors.

The result is the patch was uploaded to WordPress Trac and not bbPress Trac:

Result: https://core.trac.wordpress.org/attachment/ticket/2907/2907.diff

Member

ntwb commented Jan 25, 2016

I just tested this pull request for bbPress trac:

The ticket: https://bbpress.trac.wordpress.org/ticket/2907

The terminal commands used and output:

netweb@MacBook ~/dev/bbpress.git.wordpress.org [master]
$ git status
On branch master
Your branch is up-to-date with 'origin/master'.
Changes not staged for commit:
  (use "git add <file>..." to update what will be committed)
  (use "git checkout -- <file>..." to discard changes in working directory)

    modified:   src/templates/default/js/user.js

no changes added to commit (use "git add" and/or "git commit -a")

netweb@MacBook ~/dev/bbpress.git.wordpress.org [master]
$ grunt upload_patch:2907
Running "upload_patch:2907" (upload_patch) task
? Enter your WordPress.org username netweb
? Enter your WordPress.org password ********************************************************
Uploaded patch.

Done, without errors.

The result is the patch was uploaded to WordPress Trac and not bbPress Trac:

Result: https://core.trac.wordpress.org/attachment/ticket/2907/2907.diff

@ericandrewlewis

This comment has been minimized.

Show comment
Hide comment
@ericandrewlewis

ericandrewlewis Jan 25, 2016

Contributor

@ntwb would you expect it to upload to bbPress Trac? I don't believe patch_wordpress works for bbPress out of the box.

Contributor

ericandrewlewis commented Jan 25, 2016

@ntwb would you expect it to upload to bbPress Trac? I don't believe patch_wordpress works for bbPress out of the box.

@ntwb

This comment has been minimized.

Show comment
Hide comment
@ntwb

ntwb Jan 25, 2016

Member

By defining bbPress' Trac URL with tracUrl in bbPress' Gruntfile.js we have full support for the existing patch task, e.g. grunt patch:1234 for bbPress, likewise for BuddyPress.

See: https://bbpress.trac.wordpress.org/browser/trunk/Gruntfile.js#L211

Now, I've just defined tracUrl for the Grunt upload_patch task and it worked perfectly,I should have looked closer at the source when initially testing 馃槒

Patch all be it including the grunt task option above is now correctly added to the bbPress ticket :)

https://bbpress.trac.wordpress.org/attachment/ticket/2907/2907.diff
https://bbpress.trac.wordpress.org/ticket/2907

Member

ntwb commented Jan 25, 2016

By defining bbPress' Trac URL with tracUrl in bbPress' Gruntfile.js we have full support for the existing patch task, e.g. grunt patch:1234 for bbPress, likewise for BuddyPress.

See: https://bbpress.trac.wordpress.org/browser/trunk/Gruntfile.js#L211

Now, I've just defined tracUrl for the Grunt upload_patch task and it worked perfectly,I should have looked closer at the source when initially testing 馃槒

Patch all be it including the grunt task option above is now correctly added to the bbPress ticket :)

https://bbpress.trac.wordpress.org/attachment/ticket/2907/2907.diff
https://bbpress.trac.wordpress.org/ticket/2907

@ericandrewlewis

This comment has been minimized.

Show comment
Hide comment
@ericandrewlewis

ericandrewlewis Jan 25, 2016

Contributor

Awesome!

Contributor

ericandrewlewis commented Jan 25, 2016

Awesome!

@ntwb

This comment has been minimized.

Show comment
Hide comment
@ntwb

ntwb Feb 22, 2016

Member

Is there anything left to do here?

Member

ntwb commented Feb 22, 2016

Is there anything left to do here?

@ericandrewlewis

This comment has been minimized.

Show comment
Hide comment
@ericandrewlewis

ericandrewlewis Feb 22, 2016

Contributor

@ntwb merge ;)

Contributor

ericandrewlewis commented Feb 22, 2016

@ntwb merge ;)

@aaronjorbin

This comment has been minimized.

Show comment
Hide comment
@aaronjorbin

aaronjorbin Feb 23, 2016

Member

One thing this is lacking is documentation for the readme.

Member

aaronjorbin commented Feb 23, 2016

One thing this is lacking is documentation for the readme.

@aaronjorbin aaronjorbin referenced this pull request Feb 23, 2016

Closed

Release 0.4.0 #35

9 of 10 tasks complete
@ntwb

This comment has been minimized.

Show comment
Hide comment
@ntwb

ntwb Feb 23, 2016

Member

It is working as expected for bbPress Trac:
https://bbpress.trac.wordpress.org/ticket/2914 works as expected, patch successfully uploaded

It is working as expected for WordPress Trac
https://core.trac.wordpress.org/ticket/35917 works as expected, patch successfully uploaded

It is not working for BuddyPress Trac
https://buddypress.trac.wordpress.org/ticket/6923 I'm unable to upload the BuddyPress patch:

$ grunt upload_patch:6923
Running "upload_patch:6923" (upload_patch) task
? Enter your WordPress.org username netweb
? Enter your WordPress.org password ********************************************
Warning: Something went wrong when attempting to upload the patch. Please confirm your credentials and the ticket number. Error: XML-RPC fault: XML_RPC privileges are required to perform this operation. You don't have the required permissions. Use --force to continue.

The TracXMLRPC 1.1.2 package is enabled on bbPress Trac, I don't have admin access to BuddyPress Trac, but before pinging a BP Trac admin @ericandrewlewis is it just this package needs to be enabled on Trac for this functionality to work or is it a permissions issue not tied to said TracXMLRPC package?

Edit: Just tested with WordPress Trac and works as expected.

Member

ntwb commented Feb 23, 2016

It is working as expected for bbPress Trac:
https://bbpress.trac.wordpress.org/ticket/2914 works as expected, patch successfully uploaded

It is working as expected for WordPress Trac
https://core.trac.wordpress.org/ticket/35917 works as expected, patch successfully uploaded

It is not working for BuddyPress Trac
https://buddypress.trac.wordpress.org/ticket/6923 I'm unable to upload the BuddyPress patch:

$ grunt upload_patch:6923
Running "upload_patch:6923" (upload_patch) task
? Enter your WordPress.org username netweb
? Enter your WordPress.org password ********************************************
Warning: Something went wrong when attempting to upload the patch. Please confirm your credentials and the ticket number. Error: XML-RPC fault: XML_RPC privileges are required to perform this operation. You don't have the required permissions. Use --force to continue.

The TracXMLRPC 1.1.2 package is enabled on bbPress Trac, I don't have admin access to BuddyPress Trac, but before pinging a BP Trac admin @ericandrewlewis is it just this package needs to be enabled on Trac for this functionality to work or is it a permissions issue not tied to said TracXMLRPC package?

Edit: Just tested with WordPress Trac and works as expected.

@ericandrewlewis

This comment has been minimized.

Show comment
Hide comment
@ericandrewlewis

ericandrewlewis Feb 23, 2016

Contributor

@ntwb said

is it just this package needs to be enabled on Trac for this functionality to work or is it a permissions issue not tied to said TracXMLRPC package?

The TracXMLRPC package needs to be enabled, and the endpoint for patch uploads must be open to all users.

Contributor

ericandrewlewis commented Feb 23, 2016

@ntwb said

is it just this package needs to be enabled on Trac for this functionality to work or is it a permissions issue not tied to said TracXMLRPC package?

The TracXMLRPC package needs to be enabled, and the endpoint for patch uploads must be open to all users.

@ericandrewlewis

This comment has been minimized.

Show comment
Hide comment
@ericandrewlewis

ericandrewlewis Feb 23, 2016

Contributor

Added some usage docs in the readme

Contributor

ericandrewlewis commented Feb 23, 2016

Added some usage docs in the readme

@aaronjorbin aaronjorbin merged commit e1ffac9 into WordPress:master Apr 2, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment