-
Notifications
You must be signed in to change notification settings - Fork 32
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
Add --telemetry
argument and ensure running without telemetry is possible
#287
Changes from 5 commits
54d8318
7f2d319
f5e2f7f
d56fd4d
8e4fb8b
17b5a9c
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 |
---|---|---|
|
@@ -9,6 +9,7 @@ script: | |
- yarn lint | ||
- yarn test | ||
- yarn test:integration | ||
- yarn test:notelemetry | ||
|
||
after_success: | ||
- yarn coveralls |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,8 +2,12 @@ | |
'use strict'; | ||
const { gatherTelemetryForUrl, analyzeEmberObject } = require('ember-codemods-telemetry-helpers'); | ||
|
||
const argv = require('yargs').argv; | ||
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. Should probably configure more parsing so we can give nicer error messages when the correct patterns aren't setup, also can emit 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 that w/in the scope of this PR though? 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. Yes @tomwayson, the yargs parsing was brought as part of this PR, I will make the corrections accordingly 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. Ya, I'm fine if we punt the |
||
|
||
(async () => { | ||
await gatherTelemetryForUrl(process.argv[2], analyzeEmberObject); | ||
if (argv.telemetry) { | ||
await gatherTelemetryForUrl(argv.telemetry, analyzeEmberObject); | ||
} | ||
|
||
require('codemod-cli').runTransform( | ||
__dirname, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,20 @@ | ||
# EditorConfig helps developers define and maintain consistent | ||
# coding styles between different editors and IDEs | ||
# editorconfig.org | ||
|
||
root = true | ||
|
||
|
||
[*] | ||
end_of_line = lf | ||
charset = utf-8 | ||
trim_trailing_whitespace = true | ||
insert_final_newline = true | ||
indent_style = space | ||
indent_size = 2 | ||
|
||
[*.hbs] | ||
insert_final_newline = false | ||
|
||
[*.{diff,md}] | ||
trim_trailing_whitespace = false |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
{ | ||
/** | ||
Ember CLI sends analytics information by default. The data is completely | ||
anonymous, but there are times when you might want to disable this behavior. | ||
|
||
Setting `disableAnalytics` to true will prevent any data from being sent. | ||
*/ | ||
"disableAnalytics": false | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,16 @@ | ||
# unconventional js | ||
/blueprints/*/files/ | ||
/vendor/ | ||
|
||
# compiled output | ||
/dist/ | ||
/tmp/ | ||
|
||
# dependencies | ||
/bower_components/ | ||
|
||
# misc | ||
/coverage/ | ||
|
||
# ember-try | ||
/.node_modules.ember-try/ |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,39 @@ | ||
module.exports = { | ||
root: true, | ||
parserOptions: { | ||
ecmaVersion: 2017, | ||
sourceType: 'module' | ||
}, | ||
plugins: [ | ||
'ember' | ||
], | ||
extends: [ | ||
'eslint:recommended', | ||
'plugin:ember/recommended' | ||
], | ||
env: { | ||
browser: true | ||
}, | ||
rules: { | ||
}, | ||
overrides: [ | ||
// node files | ||
{ | ||
files: [ | ||
'ember-cli-build.js', | ||
'testem.js', | ||
'blueprints/*/index.js', | ||
'config/**/*.js', | ||
'lib/*/index.js' | ||
], | ||
parserOptions: { | ||
sourceType: 'script', | ||
ecmaVersion: 2015 | ||
}, | ||
env: { | ||
browser: false, | ||
node: true | ||
} | ||
} | ||
] | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,23 @@ | ||
# See https://help.github.com/ignore-files/ for more about ignoring files. | ||
|
||
# compiled output | ||
/dist/ | ||
/tmp/ | ||
|
||
# dependencies | ||
/bower_components/ | ||
/node_modules/ | ||
|
||
# misc | ||
/.sass-cache | ||
/connect.lock | ||
/coverage/ | ||
/libpeerconnection.log | ||
/npm-debug.log* | ||
/testem.log | ||
/yarn-error.log | ||
|
||
# ember-try | ||
/.node_modules.ember-try/ | ||
/bower.json.ember-try | ||
/package.json.ember-try |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,26 @@ | ||
--- | ||
language: node_js | ||
node_js: | ||
- "6" | ||
|
||
sudo: false | ||
dist: trusty | ||
|
||
addons: | ||
chrome: stable | ||
|
||
cache: | ||
directories: | ||
- $HOME/.npm | ||
|
||
env: | ||
global: | ||
# See https://git.io/vdao3 for details. | ||
- JOBS=1 | ||
|
||
before_install: | ||
- npm config set spin false | ||
|
||
script: | ||
- npm run lint:js | ||
- npm test |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
{ | ||
"ignore_dirs": ["tmp", "dist"] | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,56 @@ | ||
# input | ||
|
||
This README outlines the details of collaborating on this Ember application. | ||
A short introduction of this app could easily go here. | ||
|
||
## Prerequisites | ||
|
||
You will need the following things properly installed on your computer. | ||
|
||
* [Git](https://git-scm.com/) | ||
* [Node.js](https://nodejs.org/) (with npm) | ||
* [Ember CLI](https://ember-cli.com/) | ||
* [Google Chrome](https://google.com/chrome/) | ||
|
||
## Installation | ||
|
||
* `git clone <repository-url>` this repository | ||
* `cd input` | ||
* `npm install` | ||
|
||
## Running / Development | ||
|
||
* `ember serve` | ||
* Visit your app at [http://localhost:4200](http://localhost:4200). | ||
* Visit your tests at [http://localhost:4200/tests](http://localhost:4200/tests). | ||
|
||
### Code Generators | ||
|
||
Make use of the many generators for code, try `ember help generate` for more details | ||
|
||
### Running Tests | ||
|
||
* `ember test` | ||
* `ember test --server` | ||
|
||
### Linting | ||
|
||
* `npm run lint:js` | ||
* `npm run lint:js -- --fix` | ||
|
||
### Building | ||
|
||
* `ember build` (development) | ||
* `ember build --environment production` (production) | ||
|
||
### Deploying | ||
|
||
Specify what it takes to deploy your app. | ||
|
||
## Further Reading / Useful Links | ||
|
||
* [ember.js](https://emberjs.com/) | ||
* [ember-cli](https://ember-cli.com/) | ||
* Development Browser Extensions | ||
* [ember inspector for chrome](https://chrome.google.com/webstore/detail/ember-inspector/bmdblncegkenkacieihfhpjfppoconhi) | ||
* [ember inspector for firefox](https://addons.mozilla.org/en-US/firefox/addon/ember-inspector/) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,14 @@ | ||
import Application from '@ember/application'; | ||
import Resolver from './resolver'; | ||
import loadInitializers from 'ember-load-initializers'; | ||
import config from './config/environment'; | ||
|
||
const App = Application.extend({ | ||
modulePrefix: config.modulePrefix, | ||
podModulePrefix: config.podModulePrefix, | ||
Resolver | ||
}); | ||
|
||
loadInitializers(App, config.modulePrefix); | ||
|
||
export default App; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,4 @@ | ||
import Component from '@ember/component'; | ||
|
||
export default Component.extend({ | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
import { helper } from '@ember/component/helper'; | ||
|
||
export function watWat() { | ||
return 'wat-wat'; | ||
} | ||
|
||
export default helper(watWat); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
import { helper } from '@ember/component/helper'; | ||
|
||
export function bizBaz(params/*, hash*/) { | ||
return params; | ||
} | ||
|
||
export default helper(bizBaz); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
import { helper } from '@ember/component/helper'; | ||
|
||
export function watWat() { | ||
return 'wat-wat'; | ||
} | ||
|
||
export default helper(watWat); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
import { helper } from '@ember/component/helper'; | ||
|
||
export function beeBop() { | ||
return 'bee-bop'; | ||
} | ||
|
||
export default helper(beeBop); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,25 @@ | ||
<!DOCTYPE html> | ||
<html> | ||
<head> | ||
<meta charset="utf-8"> | ||
<meta http-equiv="X-UA-Compatible" content="IE=edge"> | ||
<title>App</title> | ||
<meta name="description" content=""> | ||
<meta name="viewport" content="width=device-width, initial-scale=1"> | ||
|
||
{{content-for "head"}} | ||
|
||
<link integrity="" rel="stylesheet" href="{{rootURL}}assets/vendor.css"> | ||
<link integrity="" rel="stylesheet" href="{{rootURL}}assets/app.css"> | ||
|
||
{{content-for "head-footer"}} | ||
</head> | ||
<body> | ||
{{content-for "body"}} | ||
|
||
<script src="{{rootURL}}assets/vendor.js"></script> | ||
<script src="{{rootURL}}assets/app.js"></script> | ||
|
||
{{content-for "body-footer"}} | ||
</body> | ||
</html> |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
import Resolver from 'ember-resolver'; | ||
|
||
export default Resolver; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,12 @@ | ||
import EmberRouter from '@ember/routing/router'; | ||
import config from './config/environment'; | ||
|
||
const Router = EmberRouter.extend({ | ||
location: config.locationType, | ||
rootURL: config.rootURL | ||
}); | ||
|
||
Router.map(function() { | ||
}); | ||
|
||
export default Router; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
{{site-header user=this.user class=(if this.user.isAdmin "admin")}} | ||
|
||
{{#super-select selected=this.user.country as |s|}} | ||
{{#each this.availableCountries as |country|}} | ||
{{#s.option value=country}}{{country.name}}{{/s.option}} | ||
{{/each}} | ||
{{/super-select}} | ||
|
||
{{ui/button text="Click me"}} |
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.
We should ask for
--no-telemetry
to explicitly opt-out of telemetry gathering.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.
That makes sense given that you want telemetry to be the default use case.
However, personally I like the current choice to have
--telemetry=
replace--url=
. To me if you don't provide--telemetry=
, then--no-telemetry
is not needed unless you wanted to use a default URL for telemetry - is that what you are suggesting?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.
I agree with @tomwayson, also having too many options/flags, for one thing, seems to bring in extra complexity
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.
When using
yargs
you get--no-telemetry
"for free" when you configure--telemetry
to mean something. I think it is perfectly fine for the implementation to not require--telemetry
or--no-telemetry
(e.g. assume--no-telemetry
if there is no--telemetry
value provided) but our docs should explicitly pass the argument.