Skip to:
Content

BuddyPress.org

Opened 9 years ago

Closed 5 months ago

Last modified 5 months ago

#6382 closed enhancement (fixed)

Add dynamic Grunt Watch task

Reported by: netweb's profile netweb Owned by: espellcaste's profile espellcaste
Milestone: 14.0.0 Priority: normal
Severity: normal Version:
Component: Build/Test Tools Keywords: has-patch
Cc:

Description

Add a new Grunt task watch to dynamically update CSS and JS assets in BP's /src folder via grunt watch

The attached patch re-implements much of what we removed in r8954

Note: This watch task differs from WordPress & bbPress as no minified assets are built or copied to the /build folder.

Needs rigorous testing and auditing to ensure correct number of files and correct filenames are correctly copied and built to the /build folder during grunt release

Attachments (6)

6382.diff (4.7 KB) - added by netweb 9 years ago.
build-6382.diff (133.8 KB) - added by netweb 9 years ago.
6382.3.patch (4.9 KB) - added by DJPaul 9 years ago.
6382.4.patch (5.1 KB) - added by netweb 7 years ago.
6382.5.patch (5.5 KB) - added by DJPaul 7 years ago.
refresh for trunk
6382.6.patch (6.2 KB) - added by DJPaul 7 years ago.

Download all attachments as: .zip

Change History (44)

@netweb
9 years ago

@netweb
9 years ago

#1 @netweb
9 years ago

Diff of the /build folder via grunt release: (before and after of this tickets 6382.diff patch)

$ git status
On branch master
Changes not staged for commit:
  (use "git add/rm <file>..." to update what will be committed)
  (use "git checkout -- <file>..." to discard changes in working directory)

	modified:   bp-activity/admin/js/admin.min.js
	modified:   bp-activity/js/mentions.min.js
	deleted:    bp-core/deprecated/js/autocomplete/jquery.autocomplete.min.js
	deleted:    bp-core/deprecated/js/autocomplete/jquery.autocompletefb.min.js
	deleted:    bp-core/deprecated/js/autocomplete/jquery.bgiframe.min.js
	deleted:    bp-core/deprecated/js/autocomplete/jquery.dimensions.min.js
	modified:   bp-core/js/avatar.min.js
	modified:   bp-core/js/bp-plupload.min.js
	modified:   bp-core/js/confirm.min.js
	modified:   bp-core/js/jquery-cookie.min.js
	modified:   bp-core/js/jquery-query.min.js
	modified:   bp-core/js/jquery-scroll-to.min.js
	deleted:    bp-core/js/jquery.atwho.min.js
	deleted:    bp-core/js/jquery.caret.min.js
	modified:   bp-core/js/webcam.min.js
	modified:   bp-core/js/widget-members.min.js
	modified:   bp-friends/js/widget-friends.min.js
	modified:   bp-groups/admin/js/admin.min.js
	modified:   bp-groups/js/widget-groups.min.js
	modified:   bp-members/admin/js/admin.min.js
	deleted:    bp-templates/bp-legacy/css/twentyfifteen.scss
	modified:   bp-templates/bp-legacy/js/buddypress.min.js
	modified:   bp-templates/bp-legacy/js/password-verify.min.js
	modified:   bp-xprofile/admin/js/admin.min.js
	modified:   buddypress.pot

Untracked files:
  (use "git add <file>..." to include in what will be committed)

	bp-core/deprecated/js/autocomplete/jquery.min.js
	bp-core/js/jquery.min.js
	bp-forums/bbpress/bb-admin/js/admin-forums.min.js
	bp-forums/bbpress/bb-admin/js/common.min.js
	bp-forums/bbpress/bb-admin/js/utils.min.js
	bp-forums/bbpress/bb-includes/js/jquery/hoverIntent.min.js
	bp-forums/bbpress/bb-includes/js/jquery/interface.min.js
	bp-forums/bbpress/bb-includes/js/jquery/jquery.min.js
	bp-forums/bbpress/bb-includes/js/jquery/password-strength-meter.min.js
	bp-forums/bbpress/bb-includes/js/profile-edit.min.js
	bp-forums/bbpress/bb-includes/js/topic.min.js
	bp-forums/bbpress/bb-includes/js/wp-ajax-response.min.js
	bp-forums/bbpress/bb-includes/js/wp-lists.min.js

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

The full diff changeset is attached in build-6382.diff, it looks like there are some actual fixes here and some breakage, will have to just dig though this closely :)

#2 @DJPaul
9 years ago

  • Milestone changed from Awaiting Review to 2.3

I don't have enough time to shepherd this into 2.3, but if anyone else does, great. Otherwise, I'll look at it for 2.4.

#3 @DJPaul
9 years ago

  • Milestone changed from 2.3 to 2.4

#4 @DJPaul
9 years ago

  • Keywords early added

This ticket was mentioned in Slack in #buddypress by netweb. View the logs.


9 years ago

#6 @r-a-y
9 years ago

FYI, I'm using a simplified version of netweb's patch just for companion stylesheet dev at the moment :)

This ticket was mentioned in Slack in #buddypress by netweb. View the logs.


9 years ago

#8 @DJPaul
9 years ago

Taking a look at this. Notes:

  • Patch removes SASS files from build. (this is OK)
  • Patch removes extDot: 'last', for uglify, from the old patch. Not sure why. (Why?)
Last edited 9 years ago by DJPaul (previous) (diff)

#9 @DJPaul
9 years ago

Goal:
Run equivalent tasks from grunt commit for each of the watch categories when a file changes.

Problems:

  • When watch js, making an invalid change to buddypress.js (removing a semi-colon, etc), is not caught. For jsvalidate and jshint, it says "0 files are valid" which I take to mean it has checked 0 files, because the output is not a warning.
  • grunt watch gets caught in some kind of runaway loop.
  • grunt watch:all copies the intention from WordPress, which is to copy those files into the build folder. BP doesn't want that.
  • grunt watch:styles needs to run on all changed SASS in the bp-legacy (but not .CSS), and needs to run on .CSS everywhere else. It also needs all the tasks added to it: scsslint, sass, cssjanus. (same as from the src task)
  • This is a proper PITA to work on.

@DJPaul
9 years ago

#10 @DJPaul
9 years ago

  • Keywords needs-patch added; needs-testing has-patch dev-feedback early removed
  • Milestone changed from 2.4 to Future Release

I've attached my latest efforts, 6382.3.patch, but needs much more work to fix the issues to enable more testing.

Last edited 9 years ago by DJPaul (previous) (diff)

#11 @netweb
8 years ago

#7021 was marked as a duplicate

#12 @DJPaul
8 years ago

  • Component changed from Tools - Build Process to Build/Test Tools

@netweb
7 years ago

#13 @netweb
7 years ago

  • Milestone changed from Future Release to 3.0
  • Owner set to netweb
  • Status changed from new to assigned

In 6382.4.patch is a rudimentary refresh of 6382.3.patch with a couple of tweaks to get started, now to iterate further and address the concerns already noted in this ticket.

Moving to 3.0 to get this watch task in finally

See also: #7551 and cc @hnla

This ticket was mentioned in Slack in #buddypress by netweb. View the logs.


7 years ago

#15 @espellcaste
7 years ago

  • Keywords has-patch added; needs-patch removed

#16 @hnla
7 years ago

@netweb I'll patch with latest and test the workflow a bit later for the Nouveau aspects.

#17 @hnla
7 years ago

@netweb some quick reports on a patched gruntfile and running tasks:

Watch seems not to be picking up changes on the partials in /common-styles/ - not looked closely at why so may be something obvious or that I've done.
Yep seemed to be my issue, watch task running ok.

edit/ no it's not :0 odd behaviour? It's not running on subsequent changes to partial files, didn't on the first file changed but then trying again a bit later did kick watch into life, now it's stopped again, not sure where the issue lies, suspect my end but no time to look deeply at the moment.

Running 'src' task highlights a possible issue where I have a additional template pack dir under /bp-templates/ while that is disabled as such, tasks do see this and try and jshint/stylelint it's files.

At the moment it's linting the compiled '.css' file so maybe we need to do:

'!bp-templates/*/css/buddypress.css'

instead of:
'!bp-templates/bp-nouveau/css/buddypress.css'

wild card the template packs dirs where we exclude ( and possibly for sass too)?

The result of src task appears to compile the rtl file but not the .css one after a change to one of the partial scss files - this may be due to not seeing the change and running the buddypress.scss compile, it's something like that.

Last edited 7 years ago by hnla (previous) (diff)

This ticket was mentioned in Slack in #buddypress by hnla. View the logs.


7 years ago

@DJPaul
7 years ago

refresh for trunk

#19 @DJPaul
7 years ago

The latest state of this patch is not minifying the RTL CSS files. I'm looking into this with the aim to having it committed.

(EDIT: 6.patch should fix this)

Last edited 7 years ago by DJPaul (previous) (diff)

@DJPaul
7 years ago

#20 @DJPaul
7 years ago

@netweb Can you spare up this January please to get this done once and for all? If you can't, please let us know. I don't think Hugo and I have enough collective Grunt knowledge to make this work. Otherwise, this ticket is moving back to Future Release...

6.patch is the latest iteration. I've fixed a few things that looked wrong, and also removed rtlcss:dynamic because I don't think we need that.

(For clarity, we don't need to watch the build folder for changes, only src. To remind, some tasks only run on the build folder, which is fine).

#21 @DJPaul
6 years ago

  • Keywords needs-patch added; has-patch removed

#22 @DJPaul
6 years ago

  • Milestone changed from 3.0 to Awaiting Contributions

#23 @mercime
4 years ago

  • Milestone changed from Awaiting Contributions to 7.0.0

We need this esp for the Twenty * stylesheets

#24 @imath
4 years ago

  • Milestone changed from 7.0.0 to Up Next

Let's try to work on this during next development cycle

#25 @imath
19 months ago

  • Milestone changed from Up Next to 12.0.0

#26 @imath
13 months ago

  • Milestone changed from 12.0.0 to Awaiting Contributions

#27 @dhrumilk
13 months ago

  • Keywords has-patch added; needs-patch removed

#28 @iandunn
9 months ago

An alternative approach would be to setup phpunit-watcher or Pest.

phpunit-watcher is used in Gutenberg, wordcamp.org, and several other wporg repos. They've merged several PRs I've sent when I ran into bugs, but it doesn't seem to be as well maintained as Pest. Pest uses Jest-like syntax, which I don't personally prefer, but that may just be because I'm a grumpy old man :) It looks like it'll also work with traditional PHPUnit tests, though, so nothing has to be migrated.

I imagine either would be much easier to work with than Grunt, which is usually unpleasant in my experience.

I think the most recent time I set up phpunit-watcher was for wporg-two-factor, so that might be the best config to reference:

https://github.com/WordPress/wporg-two-factor/blob/c7b22f288d4943410e9a3cc301e6d1c4461575eb/phpunit-watcher.yml.dist

https://github.com/WordPress/wporg-two-factor/blob/c7b22f288d4943410e9a3cc301e6d1c4461575eb/composer.json#L41-L44

See also https://github.com/WordPress/gutenberg/pull/27058
Related: https://core.trac.wordpress.org/ticket/50412

This ticket was mentioned in PR #172 on buddypress/buddypress by @iandunn.


9 months ago
#29

This is an alternative approach for a test watcher to the Grunt approach in 6382.6.patch. phpunit-watcher is used in Gutenberg, wordcamp.org, and several other wporg repos.

Pest may be a better alternative long term, but that might require a more substantial migration. There isn't any watcher right now, which makes testing less convenient (and therefore less likely). The ticket has been dormant for a long time, and this only took a few minutes to set up, so it seems like a good short/medium-term solution to me, even if folks eventually want to switch to Pest or something else. IMO it'd also be fine as a long-term solution.

To test, run composer update, then something like composer run test:watch -- --group=group,cache. It should re-run the selected tests automatically when any PHP files in src or tests change.

renatonascalves commented on PR #172:


5 months ago
#30

I like this. I use phpunit-watcher locally, but using a different configuration.

@iandunn Would you be willing to resolve the conflicts and make sure this file is not bundled to wp?

@imath any objection adding this to core plugin? It's mostly developer focused.

#31 @espellcaste
5 months ago

  • Milestone changed from Awaiting Contributions to 14.0.0
  • Owner changed from netweb to espellcaste

imath commented on PR #172:


5 months ago
#32

@renatonascalves sure no problem. Sorry @iandunn it got out of my radar 😞

renatonascalves commented on PR #172:


5 months ago
#34

Closing it in favor of #233

dcavins commented on PR #233:


5 months ago
#35

I'm interested to see what this does!

renatonascalves commented on PR #233:


5 months ago
#36

@dcavins Did you try it? It essentially restarts the unit tests if you make a change in php files. This is a developer-focused tool.

#37 @espellcaste
5 months ago

  • Resolution set to fixed
  • Status changed from assigned to closed

In 13739:

Adding phpunit-watcher. A tool to automatically rerun PHPUnit tests when source code changes.

This is a useful tool in the development of BuddyPress.

Props iandunn
fixes #6382
closes https://github.com/buddypress/buddypress/pull/233

#38 @espellcaste
5 months ago

In 13740:

Adding the phpunit-watcher config file.

see #6382

Note: See TracTickets for help on using tickets.