-
Notifications
You must be signed in to change notification settings - Fork 1
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
Feature/hackathon may 18 migrate to es6 and make npm #1
base: develop
Are you sure you want to change the base?
Changes from 15 commits
4ab801a
c9f1a59
65cbf90
7a3083d
3455d78
7f85858
fff7691
963d7bd
1c1997b
175488e
87f96e6
528c4ef
ee66e11
20f246b
bb876e7
35e9f21
38307c5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,4 @@ | ||
{ | ||
"presets": ["es2015", "react"], | ||
"plugins": ["transform-es2015-modules-commonjs","dynamic-import-webpack"] | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
/coverage/* | ||
/versions/* | ||
/dist/* |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,28 @@ | ||
{ | ||
"env": { | ||
"browser": true, | ||
"node": false | ||
}, | ||
"parser": "babel-eslint", | ||
"extends": ["eslint:recommended", "standard"], | ||
"rules": { | ||
"no-console": 0, | ||
"no-useless-call": 0, | ||
"no-unused-vars": ["error", { "args": "after-used" }], | ||
"indent": ["error", 4, { "SwitchCase": 1 }], | ||
"semi": ["error", "always"], | ||
"padded-blocks": 0, | ||
"valid-jsdoc": ["error", { | ||
"requireParamDescription": false, | ||
"requireReturnDescription": false | ||
}], | ||
"require-jsdoc": ["error", { | ||
"require": { | ||
"FunctionDeclaration": true, | ||
"MethodDefinition": true, | ||
"ClassDeclaration": true, | ||
"ArrowFunctionExpression": false | ||
} | ||
}] | ||
} | ||
} |
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,40 +1,67 @@ | ||
var APPROVED_ORIGINS_KEY = "wdc_approved_origins"; | ||
var SEPARATOR = ","; | ||
var Cookies = require('cookies-js'); | ||
import * as Cookies from 'cookies-js'; | ||
|
||
function _getApprovedOriginsValue() { | ||
var result = Cookies.get(APPROVED_ORIGINS_KEY); | ||
return result; | ||
} | ||
const SEPARATOR = ','; | ||
export const APPROVED_ORIGINS_KEY = 'wdc_approved_origins'; | ||
// alias of Cookies for testing ( and for future replacement? ) | ||
export let CookiesLib = Cookies; | ||
|
||
/** | ||
* @returns {*} | ||
*/ | ||
function _getApprovedOriginsValue () { | ||
let result = CookiesLib.get(APPROVED_ORIGINS_KEY); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is there any reason to have an assignation here? If There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yup, good point. The 2 reasons are:
|
||
|
||
function _saveApprovedOrigins(originArray) { | ||
var newOriginString = originArray.join(SEPARATOR); | ||
console.log("Saving approved origins '" + newOriginString + "'"); | ||
|
||
// We could potentially make this a longer term cookie instead of just for the current session | ||
var result = Cookies.set(APPROVED_ORIGINS_KEY, newOriginString); | ||
return result; | ||
return result; | ||
} | ||
|
||
// Adds an approved origins to the list already saved in a session cookie | ||
function addApprovedOrigin(origin) { | ||
if (origin) { | ||
var origins = getApprovedOrigins(); | ||
origins.push(origin); | ||
_saveApprovedOrigins(origins); | ||
} | ||
/** | ||
* | ||
* @param {Array<String>} originArray | ||
* @returns {*} | ||
*/ | ||
function _saveApprovedOrigins (originArray) { | ||
|
||
const APPROVED_ORIGINS = originArray.join(SEPARATOR); | ||
|
||
console.log(`Saving approved origins ${APPROVED_ORIGINS} `); | ||
|
||
// We could potentially make this a longer term cookie instead of just for the current session | ||
let result = CookiesLib.set(APPROVED_ORIGINS_KEY, APPROVED_ORIGINS); | ||
|
||
return result; | ||
} | ||
|
||
// Retrieves the origins which have already been approved by the user | ||
function getApprovedOrigins() { | ||
var originsString = _getApprovedOriginsValue(); | ||
if (!originsString || 0 === originsString.length) { | ||
return []; | ||
} | ||
/** | ||
* Adds an approved origins to the list already saved in a session cookie | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think origins should be singular There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. good catch There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done |
||
* @param {String} origin | ||
* @returns {Object} | ||
*/ | ||
export function addApprovedOrigin (origin) { | ||
|
||
if (origin) { | ||
let origins = getApprovedOrigins(); | ||
|
||
origins.push(origin); | ||
|
||
// pass along the output of the private function | ||
return _saveApprovedOrigins(origins); | ||
} | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It might be better to return something to comply with the JSDoc (or change the JSDoc which I know is a pending task) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. you're right, I'll change it to {Object|Undefined} There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done |
||
var origins = originsString.split(SEPARATOR); | ||
return origins; | ||
} | ||
|
||
module.exports.addApprovedOrigin = addApprovedOrigin; | ||
module.exports.getApprovedOrigins = getApprovedOrigins; | ||
/** | ||
* Retrieves the origins which have already been approved by the user | ||
* @returns {Array<String>} | ||
*/ | ||
export function getApprovedOrigins () { | ||
|
||
let originsString = _getApprovedOriginsValue(); | ||
|
||
if (!originsString || originsString.length === 0) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It seems like There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yup, same thing here, just trying to keep as much as possible untouched |
||
return []; | ||
} | ||
|
||
let origins = originsString.split(SEPARATOR); | ||
|
||
return origins; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,50 @@ | ||
/* eslint-env node, mocha, jest */ | ||
import * as ApprovedOrigins from './ApprovedOrigins'; | ||
|
||
// use if required for debugging | ||
let consoleLog = console.log; // eslint-disable-line no-unused-vars | ||
console.log = jest.fn(); | ||
|
||
describe('UNIT - ApprovedOrigins', () => { | ||
|
||
beforeEach(() => { | ||
// Cookies Mock to make this a Unit Test instead of an integration test | ||
// temp mock, will investigate on a better mocking for Cookies lib :( | ||
// (tried with standard jest mock, ridiculously failing) | ||
|
||
ApprovedOrigins.CookiesLib.mokedCookies = {}; | ||
|
||
ApprovedOrigins.CookiesLib.get = jest.fn(() => { | ||
return ApprovedOrigins.CookiesLib.mokedCookies[ApprovedOrigins.APPROVED_ORIGINS_KEY]; | ||
}); | ||
|
||
ApprovedOrigins.CookiesLib.set = jest.fn((key, val) => { | ||
ApprovedOrigins.CookiesLib.mokedCookies[key] = val; | ||
|
||
return ApprovedOrigins.CookiesLib; | ||
}); | ||
|
||
}); | ||
|
||
it('getApprovedOrigins should get an empty array if no value is set', () => { | ||
expect(ApprovedOrigins.getApprovedOrigins()).toEqual([]); | ||
}); | ||
|
||
it('addApprovedOrigin should add an approved origin and ApprovedOrigins.getApprovedOrigins get it correctly', () => { | ||
let origin = 'http://tableau.com'; | ||
expect(ApprovedOrigins.addApprovedOrigin(origin)).toBe(ApprovedOrigins.CookiesLib); | ||
|
||
expect(ApprovedOrigins.getApprovedOrigins()).toEqual([origin]); | ||
|
||
}); | ||
|
||
it('addApprovedOrigin should add multiple values and get an array of the values', () => { | ||
let origin1 = 'http://tableau.com'; | ||
let origin2 = 'http://connectors.tableab.com'; | ||
ApprovedOrigins.addApprovedOrigin(origin1); | ||
ApprovedOrigins.addApprovedOrigin(origin2); | ||
|
||
expect(ApprovedOrigins.getApprovedOrigins()).toEqual([origin1, origin2]); | ||
|
||
}); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,50 +1,75 @@ | ||
var fs = require('fs'); | ||
var path = require('path'); | ||
const execSync = require('child_process').execSync; | ||
|
||
var WDC_LIB_PREFIX = "tableauwdc-"; | ||
|
||
function VersionNumber(versionString) { | ||
var components = versionString.split("."); | ||
if (components.length < 3) { | ||
console.log() | ||
throw "Invalid number of components. versionString was '" + versionString + "'"; | ||
} | ||
|
||
this.major = parseInt(components[0]).toString(); | ||
this.minor = parseInt(components[1]).toString(); | ||
this.fix = parseInt(components[2]).toString(); | ||
} | ||
import { version as BUILD_NUMBER } from '../package.json'; | ||
export let WDC_LIB_PREFIX = 'tableauwdc-'; | ||
|
||
VersionNumber.prototype.toString = function() { | ||
return this.major + "." + this.minor + "." + this.fix; | ||
} | ||
/** | ||
* | ||
*/ | ||
export class VersionNumber { | ||
/** | ||
* | ||
* @param {String} versionString | ||
*/ | ||
constructor (versionString) { | ||
let components = versionString.split('.'); | ||
|
||
VersionNumber.prototype.compare = function(other) { | ||
var majorDiff = this.major - other.major; | ||
var minorDiff = this.minor - other.minor; | ||
var fixDiff = this.fix - other.fix; | ||
if (components.length < 3) { | ||
console.log(); | ||
throw new Error(`Invalid number of components. versionString was ${versionString} `); | ||
} | ||
|
||
if (majorDiff != 0) return majorDiff; | ||
if (minorDiff != 0) return minorDiff; | ||
if (fixDiff != 0) return fixDiff; | ||
this.major = parseInt(components[0]).toString(); | ||
this.minor = parseInt(components[1]).toString(); | ||
this.patch = parseInt(components[2]).toString(); | ||
} | ||
|
||
return 0; | ||
} | ||
/** | ||
* @returns {String} | ||
*/ | ||
toString () { | ||
return `${this.major}.${this.minor}.${this.patch}`; | ||
} | ||
|
||
/** | ||
* | ||
* @param {Object} other | ||
* @returns {Number} | ||
*/ | ||
compare (other) { | ||
let majorDiff = this.major - other.major; | ||
let minorDiff = this.minor - other.minor; | ||
let fixDiff = this.patch - other.fix; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we have a copy paste error here? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nope, patch is the correct definition for the third number on the semver definition, I left the fix word for the input to ask Ben about that, I'd go for both to be patch for consistency There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. changed |
||
|
||
if (majorDiff !== 0) { | ||
return majorDiff; | ||
} | ||
|
||
if (minorDiff !== 0) { | ||
return minorDiff; | ||
} | ||
|
||
function getBuildNumber() { | ||
// Grab the version number from the environment variable | ||
var versionNumber = process.env.npm_package_config_versionNumber; | ||
if (versionNumber) { | ||
console.log("Found versionNumber in environment variable: '" + versionNumber + "'"); | ||
} else { | ||
versionNumber = process.argv.versionNumber; | ||
console.log("Found versionNumber in argument: '" + versionNumber + "'"); | ||
} | ||
|
||
return versionNumber; | ||
if (fixDiff !== 0) { | ||
return fixDiff; | ||
} | ||
|
||
return 0; | ||
} | ||
} | ||
|
||
module.exports.VersionNumber = VersionNumber; | ||
module.exports.WDC_LIB_PREFIX = WDC_LIB_PREFIX; | ||
module.exports.getBuildNumber = getBuildNumber; | ||
/** | ||
* | ||
* @param {Object} config | ||
* @returns {String} | ||
*/ | ||
export function getBuildNumber ({ showLog = false } = {}) { | ||
// Grab the version number from the package json property ( one static source of truth ) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does it miss some code here? It is not clear how the version number is retrieved. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. you're right, it's explaining the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done |
||
|
||
if (!BUILD_NUMBER) { | ||
throw new Error(`Unable to retrieve version number from package.json`); | ||
} | ||
|
||
if (showLog) { | ||
console.log(`Found versionNumber in package.json: ${BUILD_NUMBER}`); | ||
} | ||
|
||
return BUILD_NUMBER; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
unfortunately we have to leave the no-console rule set off globally because it's part of the application flow.
I'd like to improve this later.