Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

v7.9.0 (CPD-11491) - Optional removal of console.log statements #112

Merged
merged 10 commits into from
Feb 5, 2018
12 changes: 11 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,17 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/)
and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html).


v7.9.0
------------------------------
*February 2, 2018*

### Added
- `stripDebug` option to `config.js` to allow the inclusion of console statements in production builds.
- `--noStripDebug` command line flag.

### Changed
- Jest rollback from v22.1.4 to v21.2.1.

v7.8.0
------------------------------
*February 1, 2018*
Expand All @@ -16,7 +27,6 @@ v7.8.0
### Changed
- Minor package updates


v7.7.0
------------------------------
*January 30, 2018*
Expand Down
50 changes: 38 additions & 12 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,11 @@ Gulp build tasks for use across Fozzie modules.
- [Config object](#config-object)
- [pathBuilder object](#pathbuilder-object)
- [The Gulp Tasks](#the-gulp-tasks)
- [Development only tasks](#development-only-tasks)
- [Development-only tasks](#development-only-tasks)
- [Config](#config)
- [Other config](#other-config)
- [Path Builder](#path-builder)
- [Running the unit tests](running-the-unit-tests)
- [Running the unit tests](#running-the-unit-tests)


## Setup
Expand Down Expand Up @@ -245,7 +245,7 @@ Runs the same tasks as [`watch`](#watch) as well as the following watch tasks.
Runs the [`assemble`](#assemble) task when documentation files are changed.


### Development only tasks
### Development-only tasks

- #### `docs`

Expand Down Expand Up @@ -304,7 +304,8 @@ Here is the outline of the configuration options, descriptions of each are below
],
jsDir,
lintPaths,
usePackageVersion
usePackageVersion,
stripDebug
},
img: {
imgDir,
Expand Down Expand Up @@ -391,7 +392,7 @@ Root dist directory for your assets.

Type: `boolean`

Default: true
Default: `true`

Will add a content hash to the JS and CSS filenames, generating a new filename if any of the file's contents have changed. This can be utilised to force the clients to get the latest version of an updated asset.

Expand Down Expand Up @@ -443,7 +444,9 @@ Will add a content hash to the JS and CSS filenames, generating a new filename i

Type: `boolean`

Default is set to false, when set to true this will bundle a versioned css file e.g `'filename-[version].css'`.
Default: `false`

When set to `true` this will bundle a versioned css file e.g `'filename-[version].css'`.


### `js`
Expand Down Expand Up @@ -506,8 +509,31 @@ Will add a content hash to the JS and CSS filenames, generating a new filename i

Type: `boolean`

Default is set to false, when set to true this will bundle a versioned JS file e.g `'filename-[version].js'`.
Default: `false`

When set to `true` this will bundle a versioned JS file e.g `'filename-[version].js'`.

- #### `stripDebug`

Type: `boolean`

Default: `true`

This can also be controlled using the `--noStripDebug` flag. When this flag is added, `console.log()` statements will not be removed for production builds.

#### Examples:

`gulp scripts:bundle --prod --noStripDebug`

This would generate the JS files as part of a production build, but would still include `console.log()` statements. Intended for QA releases.

`gulp scripts:bundle --prod`

This is a normal production build and would not include `console.log()` statements.

`gulp scripts:bundle --noStripDebug`

**For non-production builds, the flag has no effect: you will still get debug statements even if include the flag.**

### `img`

Expand Down Expand Up @@ -642,7 +668,7 @@ Will add a content hash to the JS and CSS filenames, generating a new filename i
In which:
- `path` is a string specifying the path within the relevant asset `src` folder of the asset to be copied.
- `dest` is a string specifying that destination folder for the asset to be copied to, within the relevant asset `dist` folder.
- `revision` is a boolean such that if it is true, the asset will be [revision hashed](https://www.npmjs.com/package/gulp-rev) when copied to it’s destination.
- `revision` is a boolean such that if it is `true`, the asset will be [revision hashed](https://www.npmjs.com/package/gulp-rev) when copied to its destination.

`path` and `dest` must always be defined for each asset you wish to copy.

Expand Down Expand Up @@ -725,13 +751,13 @@ Will add a content hash to the JS and CSS filenames, generating a new filename i

Default: `''`

Applies a base path to asset URLs when publishing documentation to Github pages. By default this is set to be an empty string.
Applies a base path to asset URLs when publishing documentation to Github pages. By default this is set to be an empty string.

- #### `helpers`

Type: `object`

Default: `{ }`
Default: `{}`

Can pass in an object set of functions, which will be exposed in handlebars as helper functions in the documentation tasks when called using their object key.

Expand Down Expand Up @@ -779,7 +805,7 @@ Will add a content hash to the JS and CSS filenames, generating a new filename i

Default: `1000`

Wait for a specified window of event-silence before sending any reload events.
Wait for a specified window of event-silence (in milliseconds) before sending any reload events.

### `misc`

Expand Down Expand Up @@ -829,7 +855,7 @@ The following options are also present in the config but cannot be overridden.

Type: `boolean`

Set to the opposite value of `isProduction`
Set to the opposite value of `isProduction`.


## Path Builder
Expand Down
4 changes: 3 additions & 1 deletion config.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ const ConfigOptions = () => {
const isProduction = !!gutil.env.prod;
const isDev = !isProduction;
const envLog = isProduction ? 'production' : 'development';
const stripDebug = !gutil.env.noStripDebug;

gutil.log(gutil.colors.yellow(`🐻 Running Gulp task for ${consumingPackageConfig.name}@${consumingPackageConfig.version} in ${gutil.colors.bold(envLog)} mode ${gutil.colors.gray(`(v${packageConfig.version})`)}`));

Expand Down Expand Up @@ -45,7 +46,8 @@ const ConfigOptions = () => {
},
jsDir: 'js',
lintPaths: [''],
usePackageVersion: false
usePackageVersion: false,
stripDebug
},

/**
Expand Down
4 changes: 2 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@justeat/gulp-build-fozzie",
"version": "7.8.0",
"version": "7.9.0",
"description": "Gulp build tasks for use across Fozzie modules",
"main": "index.js",
"author": "Damian Mullins <damian.mullins@just-eat.com> (http://www.damianmullins.com)",
Expand Down Expand Up @@ -71,7 +71,7 @@
"handlebars-helpers": "^0.10.0",
"helper-markdown": "^1.0.0",
"helper-md": "^0.2.2",
"jest-cli": "^22.1.4",
"jest-cli": "^21.2.1",
Copy link
Contributor

@DamianMullins DamianMullins Feb 2, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For future reference, one of the issues with v22.1.4 was that an error of "TypeError: environment.setup is not a function" was being thrown in some repos, there is a discussion on the Jest GitHub repo where it seems like one of the dependencies hasn't yet been updated,

"merge-stream": "^1.0.1",
"postcss-assets": "^5.0.0",
"postcss-reporter": "^5.0.0",
Expand Down
2 changes: 1 addition & 1 deletion tasks/javascript.js
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ gulp.task('scripts:bundle', () => {
))

// if production build, rip out our console logs
.pipe(gulpif(config.isProduction,
.pipe(gulpif(config.isProduction && config.js.stripDebug,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might be a silly question but why do we need to have this new option? Shouldn't we strip console logs / debuggers from production code anyway? Or is there a reason we use them in production.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a request from the OrderWeb team. They want to have production type code with debug statements for when they release to QA.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One thing that would be worth adding is the ability to set this config variable through an environment variable. So something like gulp --stripDebug=false would set this to false (as I imagine that might be how they need to set it to be different on every release server).

Copy link
Contributor Author

@xander-marjoram xander-marjoram Feb 5, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ashleynolan Should I add that in this PR?
(Verbal answer was "yes")

Fixed in latest commit.

stripDebug()
))

Expand Down
Loading