Skip to content

Fix issues identifying in-repo addon paths in getTelemetryFor. - #129

Merged
rwjblue merged 1 commit into
ember-codemods:masterfrom
rwjblue:fixup-get-telemetry-for
Jun 20, 2019
Merged

rwjblue merged 1 commit into
ember-codemods:masterfrom
rwjblue:fixup-get-telemetry-for

Conversation

@rwjblue

@rwjblue rwjblue commented Jun 20, 2019

Copy link
Copy Markdown
Member

With these changes getModulePathFor is much more context aware:

  • when inside an in-repo addon we support transforming addon and addon-test-support
  • when inside an app we support trransforming app and test

With these changes `getModulePathFor` is much more context aware:

* when inside an in-repo addon we support transforming `addon` and
  `addon-test-support`
* when inside an app we support trransforming `app` and `test`
@rwjblue
rwjblue force-pushed the fixup-get-telemetry-for branch from 90beaaf to 6ec165b Compare June 20, 2019 15:11
@rwjblue
rwjblue merged commit fd8ba6c into ember-codemods:master Jun 20, 2019
@rwjblue
rwjblue deleted the fixup-get-telemetry-for branch June 20, 2019 15:14
@rwjblue rwjblue added the bug Something isn't working label Jun 20, 2019
result = `${moduleNameRoot}/${relativePath}`;
}
} else {
if (relativePath.startsWith('addon-test-support')) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I believe addons could have files in an app/ folder, no? Also, wouldn't this ignore test files in addons?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Ah, I guess addon files in the app folder are inaccessible anyways - no way to tell if they were part of the app, or the addon 😕

@rwjblue rwjblue Jun 20, 2019 •

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@pzuraq addon's with files in the app/ folder are super problematic for us, they "appear" as if they are in the app itself (from a module name perspective) so there is no real way to map from lib/some-thing-here/app/foo/bar.js to app-name/foo/bar.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants