Page MenuHomePhabricator

MobileFrontend build does not fail when Webpack-generated files are not committed
Closed, DeclinedPublic

Description

MobileFrontend build does not fail when Webpack-generated files are not committed.

Example change that should fail: https://gerrit.wikimedia.org/r/c/mediawiki/extensions/MobileFrontend/+/491375

This was a factor in our incorrect backporting of the fix for T215101.

Event Timeline

matmarex created this task.Feb 18 2019, 9:28 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptFeb 18 2019, 9:28 PM

Change 491376 had a related patch set uploaded (by Bartosz Dziewoński; owner: Bartosz Dziewoński):
[mediawiki/extensions/MobileFrontend@master] check_bundle.sh: Always fail the build on uncommitted changes

https://gerrit.wikimedia.org/r/491376

matmarex claimed this task.Feb 18 2019, 9:38 PM
DLynch added a subscriber: DLynch.Feb 19 2019, 3:49 PM

Change 491376 abandoned by Bartosz Dziewoński:
check_bundle.sh: Always fail the build on uncommitted changes

Reason:

Patch Set 1:
This would be a great topic for our humans of the web meeting.
The current behaviour is intentional as it does the following

  • ensures it's impossible to commit compiled JS that isn't compiled from the existing source code
  • allows changes to be unstaged, which helps alleviate the need to rebase on every single commit.
  • Allows you to cherry pick patches that don't change dist into other branches and then run npm run build and amend (sadly this seems to have backfired in your experience)

The rebase problem as been also causing problems.
One thing that's been discussed is having a bot do the building so we don't have to or making sure we commit on every patch even though that leads to rebase issues.
Marking this as WIP until we've had a conversation about the trade offs of all the options available.

Okay, I am really not interested in wasting ten people's time discussing this. I had been thinking that it was a trivial oversight and that this patch would be easily accepted. I will just add this kind of check in my local development environment.

https://gerrit.wikimedia.org/r/491376

matmarex closed this task as Declined.Feb 19 2019, 11:34 PM
matmarex removed matmarex as the assignee of this task.