Skip to:
Content

BuddyPress.org

Opened 10 years ago

Closed 10 years ago

Last modified 10 years ago

#5821 closed defect (bug) (fixed)

Update grunt build tools - Fixes JS & CSS build process

Reported by: netweb's profile netweb Owned by: djpaul's profile djpaul
Milestone: 2.1 Priority: highest
Severity: blocker Version:
Component: Core Keywords: has-patch
Cc:

Description

First up, no changes in the following affect the current Grunt build tools workflow, everything remains the same.

Gruntfile.js:

  • Removes legacy grunt watch grunt tasks from cssjanus, clean, copy, variable path, grunt.event.on and grunt:config dynamic options. There is no longer any watch tasks in the BuddyPress build tools, see r8550
  • Add's grunt task option extDot: 'last', to Grunt tasks cssjanus, uglify and cssmin to allow support for files with dot/period filenames, e.g. jquery.atwho.js (previously uglify/minify this file resulted a file named jquery.min.js)
  • Removes the Grunt CSS task cssmin:rtl and cssmin:ltr with a single cssmin:minify task, we are only minifying the build folder CSS so all CSS files can be minified with a single task.
  • Added JSValidate task jsvalidate:src to validate JavaScript files in the /src folder, previously JavaScript files where only validated after running grunt build-release.
  • Grunt task grunt build now uses jsvalidate:src to verify JavaScript files in the /src folder, previously it was only validating JavaScript files in the /build folder.

CSS:

  • Fixes an issue where the RTL CSS of the file jquery.autocompletefb.css was created using the filename jquery-rtl.css, this is fixed with SVN rename so the filename is now jquery.autocompletefb-rtl.css and the associated Grunt task now fixes this with the extDot: 'last' task option.
  • CSS is simplified with BP_CSS = **/*.css for all CSS files with an exclusion list in BP_EXCLUDED_CSS which excludes RTL CSS primarily from the Grunt task cssjanus so that standard example.css files only have have RTL files with the extension of example-rtl.css created in the /src folder as part of grunt build and grunt build-commit tasks as the RTL files are committed to the /src repo of BuddyPress.

JavaScript:

  • The JavaScript file inclusion is simplified with BP_JS = **/*.js for all JavaScript files with an exclusion list in BP_EXCLUDED_JS which is then excluded in the Grunt task grunt jsint so 3rd party JavaScript files do not fail BuddyPress JSHint standards, each JavaScript file is still validated with the grunt jsvalidate task though.

JShint:

  • Two explicit JSHint exclusions are added to BuddyPress deprecated files to allow them to pass the Grunt task grunt jshint (This could be switched to include them in BP_EXCLUDED_JS and was an either/or choice so I chose as they are BuddyPress "Core" files and not 3rd party to add the JSHint exclusion.
  • src/bp-templates/bp-legacy/js/password-verify.js
  • src/bp-templates/bp-legacy/js/buddypress.js

Node Dependencies:

  • Updates grunt-contrib-imagemin from ~0.7.1 to ~0.8.0
  • Updates grunt-wp-i18n from ~0.4.6 to ~0.4.7
  • Removes unused grunt-phpunit

Line Endings:

  • All CSS and JavaScript files now include a blank line end of the file inline with coding standards.

Inline docs and whitespace:

  • A couple of minor docs formatting and updates
  • A couple whitespace and coding formatting updates

ToDo:

  • After this commit, sort the tasks alphabetically (It made the patch look like a dogs breakfast if included here)

In summary the following workflow performs the following (including order of execution):

  • grunt build:
    • Using jsvalidate:src validates all 20 JavaScript (12 Core & 8 3rd party) files in the /src folder
    • Using jshint lints all 12 BuddyPress Core JavaScript files in the /src folder
    • Using cssjanus converts all 10 BuddyPress CSS files to RTL equivalents in the /src folder
  • grunt build-commit calls grunt build tasks above and:
    • Using checktextdomain to check i18n text domain for all PHP files
    • Using imagemin minimize all images in the /src folder
  • grunt build-release calls grunt build-commit and grunt build tasks above and:
    • Using clean:all cleans the /build folder
    • Using copy:all copies all files and folders from /src to the /build folder
    • Using uglify minimizes all 20 JavaScript files (12 Core & 8 3rd party) in the /build folder
    • Using jsvalidate:build validates all 40 JavaScript files (20 Core & 20 minimized) in the /build folder
    • Using cssmin minimizes all 20 CSS files (10 LTR CSS & 10 RTL CSS) in the /build folder
    • Using makepot creates the buddypress.pot file in the root of the /build folder
    • Using exec:bbpress to SVN checkout bbPress 1.2 into the /build/bp-forums folder
    • Using exec:bpdefault to SVN checkout BP-Default into the /build/bp-themes/bp-default folder
    • Using test to run the PHPUnit Grunt task to run PHPUnit single site and multi site PHPUnit tests

Attachments (2)

5821.patch (18.6 KB) - added by netweb 10 years ago.
5821-svn-eol.patch (1.4 KB) - added by netweb 10 years ago.

Download all attachments as: .zip

Change History (10)

@netweb
10 years ago

This ticket was mentioned in IRC in #buddypress-dev by netweb. View the logs.


10 years ago

#2 follow-up: @DJPaul
10 years ago

I've only time to eyeball the changes right now, but I think it looks OK. Thanks for the patch, netweb.

The line ending and other whitespace changes are always going to be committed separately from anything else, so a separate patch for those would have been helpful, but at least with Git, we can be selective with files during commits, so this isn't a big deal.

sort the tasks alphabetically

Thanks for not mixing this up into this patch. :) The original idea was to have the tasks listed in the order that they're executed in (as much as possible, anyway) so that it's easier to read the file and better understand what's going on (without having to refer to the registerTask lines constantly).

Listing things alphabetically, especially considering that some of our commands run in SOURCE_DIR and some in BUILD_DIR, I think will make things more confusing. Therefore I don't think we'll want to do this with BuddyPress, unless the other core devs feel very extremely strongly about having this in alphabetical order.

Removes unused grunt-phpunit

Are you sure this isn't used by the phpunit task inside the test task?

#3 in reply to: ↑ 2 ; follow-up: @netweb
10 years ago

Replying to DJPaul:

I've only time to eyeball the changes right now, but I think it looks OK. Thanks for the patch, netweb.

Thanks :)

The line ending and other whitespace changes are always going to be committed separately from anything else, so a separate patch for those would have been helpful, but at least with Git, we can be selective with files during commits, so this isn't a big deal.

Due to weird cross platform end of line issues, the BuddyPress SVN repo would always compile two RTL CSS files with new line endings, not so via the Git repo which was a strange issue but moot now.

Whilst creating this patch and verifying CSS and JS file integrity it made sense, it also saved me having to revert the SVN repo constantly every time I ran the Grunt tasks testing this patch.

sort the tasks alphabetically

Thanks for not mixing this up into this patch. :) The original idea was to have the tasks listed in the order that they're executed in (as much as possible, anyway) so that it's easier to read the file and better understand what's going on (without having to refer to the registerTask lines constantly).

Listing things alphabetically, especially considering that some of our commands run in SOURCE_DIR and some in BUILD_DIR, I think will make things more confusing. Therefore I don't think we'll want to do this with BuddyPress, unless the other core devs feel very extremely strongly about having this in alphabetical order.

As you point out some tasks use both SOURCE_DIR and BUILD_DIR and the updated grunt jsvalidate task is a prime example, grunt jsvalidate:src is the first task ran and the grunt jsvalidate:build task is the eleventh.

A console output which can be quite long, I don't want to have to scroll to the top of the console to see which position the failed task is in, commit to memory, or have to read the task order to guesstimate where it might be in Grutfile.js, I'll reference the failed task by it's name and then go and find the task, creating this patch I was wasting loads (we are only talking seconds) of time scrolling up and down (and past) tasks where I kind of expected them to be, it's a trivial issue only saving seconds, but I find it illogical in not being sorted A-Z ;)

Removes unused grunt-phpunit

Are you sure this isn't used by the phpunit task inside the test task?

Yes, I see where you are coming from in thinking that though, kind of confusing and removed in [bbPress5414] and has never existed in WordPress Core yet all three projects basically all share the same Grunt PHPUnit task configuration.

The NPM package grunt-phpunit is quite a different beast and task configuration vastly different, it was looked at to fix #5627/#WP27343 but there is currently a bug that won't allow the passing of arguments to separate single site from multisite tests along with some challenging console output display issues also.

The grunt.registerMultiTask( 'phpunit' task runs both phpunit:default and phpunit:multisite tasks passing the command phpunit and args to grunt.util.spawn and actually executing the PEAR command phpunit with args, if you don't have the PEAR PHPUnit package installed and in your system path the task fails.

You could say it is similar to how grunt:exec works, a Grunt MultiTask grunt.util.spawn could probably possibly replace exec:bbpress and exec:bpdefault tasks, that's for another day though.

http://gruntjs.com/api/grunt.util#grunt.util.spawn

// The command to execute. It should be in the system path.
  cmd: commandToExecute,

#4 @djpaul
10 years ago

In 8952:

Whitespace tweaks to CSS/JS.

See #5821

#5 @djpaul
10 years ago

In 8953:

Grunt: update package.json with latest versions of libraries. Remove unused grunt-phpunit.

See #5821, props netweb

#6 @djpaul
10 years ago

  • Owner set to djpaul
  • Resolution set to fixed
  • Status changed from new to closed

In 8954:

Grunt: overhaul tasks config.

  • Removes legacy watch grunt tasks. The watch configuration was previously removed in r8550.
  • Add extDot: 'last' option to Grunt tasks cssjanus, uglify, and cssmin. Fixes support with files with dot filenames.
  • Consolidates the cssmin tasks into one.
  • Task build now correctly verifies JS in the src folder. Previously, it was only validating files inside build.
  • Fixes an issue where the RTL CSS of the file jquery.autocompletefb.css was created using the filename jquery-rtl.css.
  • Don't parse bp-legacy's JS with jshint until improvements are made.

Includes files changed as a result of this Grunt change.

Fixes #5821, props netweb.

#7 in reply to: ↑ 3 @netweb
10 years ago

Sweet :)

Replying to netweb:

Due to weird cross platform end of line issues, the BuddyPress SVN repo would always compile two RTL CSS files with new line endings, not so via the Git repo which was a strange issue but moot now.

Ahh this is the EOL SVN properties, it looks like the SVN EOL properties are manually set, incoming patch fixes this for the three files affected:

#8 @djpaul
10 years ago

In 8958:

Set svn:eol-style native on all text files.

Thanks to netweb for spotting these; see #5821

Note: See TracTickets for help on using tickets.