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

Get telemetry data #154

Open
wants to merge 9 commits into
base: master
from

Conversation

@tylerturdenpants
Copy link
Collaborator

tylerturdenpants commented Oct 3, 2019

No description provided.

@tylerturdenpants tylerturdenpants force-pushed the feature/get-telemetry-data branch 2 times, most recently from 75578e4 to c91accb Oct 25, 2019
@tylerturdenpants tylerturdenpants requested a review from rwjblue Oct 26, 2019
@tylerturdenpants tylerturdenpants marked this pull request as ready for review Oct 26, 2019
@tylerturdenpants tylerturdenpants force-pushed the feature/get-telemetry-data branch from c91accb to e01e2e6 Oct 26, 2019
@tylerturdenpants

This comment has been minimized.

Copy link
Collaborator Author

tylerturdenpants commented Oct 26, 2019

@rwjblue this is blocked on a 1.0.1 release of ember-codemods-telemetry-helpers Thanks!

@rwjblue

This comment has been minimized.

Copy link
Member

rwjblue commented Oct 28, 2019

Should be ready to rebase and bump to the more recent telemetry helpers

@tylerturdenpants tylerturdenpants force-pushed the feature/get-telemetry-data branch from e01e2e6 to 990098f Oct 28, 2019
@tylerturdenpants

This comment has been minimized.

Copy link
Collaborator Author

tylerturdenpants commented Oct 28, 2019

Before we release this, I need to update the readme

README.md Outdated Show resolved Hide resolved
bin/telemetry.js Outdated Show resolved Hide resolved
bin/telemetry.js Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
@tylerturdenpants tylerturdenpants requested a review from rwjblue Oct 28, 2019
@tylerturdenpants

This comment has been minimized.

Copy link
Collaborator Author

tylerturdenpants commented Oct 29, 2019

I think I want to add some test coverage to all of this. @rwjblue It’s up to you if you want to merge.

package.json Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
@@ -0,0 +1,53 @@
/* eslint-disable no-console */

const { spawn } = require('child_process');

This comment has been minimized.

Copy link
@rwjblue

rwjblue Oct 30, 2019

Member

This seems very similar to the test infrastructure code that @NullVoxPopuli recently added to ember-codemod-telemetry-helpers, maybe we can share?

This comment has been minimized.

Copy link
@tylerturdenpants

tylerturdenpants Oct 30, 2019

Author Collaborator

Ya, I was talking with @NullVoxPopuli in Discord about that. I think we can let this stay here for this PR and come back with a shared solution.

transforms/angle-brackets/index.js Outdated Show resolved Hide resolved
transforms/angle-brackets/index.js Outdated Show resolved Hide resolved
transforms/angle-brackets/index.js Outdated Show resolved Hide resolved
transforms/angle-brackets/index.js Outdated Show resolved Hide resolved
@tylerturdenpants tylerturdenpants force-pushed the feature/get-telemetry-data branch from e7a2e66 to 0f3b142 Oct 30, 2019
@tylerturdenpants tylerturdenpants requested a review from rwjblue Nov 1, 2019
@rwjblue
rwjblue approved these changes Nov 2, 2019
@@ -27,8 +28,9 @@ function getOptions() {
}

module.exports = function(file) {
let helpers = getHelperData(getTelemetry());

This comment has been minimized.

Copy link
@rwjblue

rwjblue Nov 2, 2019

Member

FWIW, I think we also want to know the names of the components too, don't we? Seems fine for a follow up, but I do think there are scenarios that we can make better with this.

For example, with the following template when {{foo}} is not a helper how do we know if it is a local property (and should be left alone) or a component invocation (which should be transformed)?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.