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

Embroider: Port to v2 format: part 2: separate docs-app #62

Merged
merged 18 commits into from
Jan 10, 2025

Conversation

bantic
Copy link
Contributor

@bantic bantic commented Jan 9, 2025

Follow-up from #61. This adds a new pkg to the monorepo: the docs app, following the embroider guide section here: https://github.com/embroider-build/embroider/blob/main/docs/porting-addons-to-v2.md#part-2-optional-split-docs-from-tests

I'm opening for review. This is best reviewed by looking at the relative diff to #61: bantic/embroider-native...bantic/embroider-native-v2-docs-app

I'll rebase it after #61 is merged.

The tldr:

  • Follows roughly the same process as in Embroider: Port to v2 format: Part 1: separate test-app #61, except this time we duplicate the test-app into docs-app and tailor each to have only the specific things (deps, templates, tests) it needs
  • Adds a few very basic "does this render" tests for the docs-app
  • Upgrades eslint to latest (v8 -> v9) for the test-app and docs-app, which requires creating a wholly new eslint config (details at https://eslint.org/docs/latest/use/migrate-to-9.0.0) -- the ember addon output repo uses eslint v9 so this is mostly just files copied from there
  • Addresses some QUnit-related lint failures now bc the ember-addon-output includes the qunit plugin for eslint. This was mostly mechanical: change assert.ok -> assert.true, change assert.equal -> assert.strictEqual. And in one case change an assertion that was called from a conditional to be called uncondtionally
  • Add the eslint-related devdeps to the addon/package.json. These should have been in Embroider: Port to v2 format: Part 1: separate test-app #61 but they weren't (and weren't noticed because linting continued to work due to yarn-classic's package hoisting)

@bantic bantic force-pushed the bantic/embroider-native-v2-docs-app branch from 873e280 to 6c80979 Compare January 10, 2025 10:06
@bantic bantic changed the title Embroider: Port to v2 format: separate docs-app Embroider: Port to v2 format: part 2: separate docs-app Jan 10, 2025
@bantic bantic marked this pull request as ready for review January 10, 2025 10:11
@bantic bantic requested a review from a team January 10, 2025 10:11
@mixonic
Copy link
Member

mixonic commented Jan 10, 2025

Not a blocker, but I am curious how we can more consistently leverage Addepar's own linting/style defaults in this kind of codebase without it being a headache.

@bantic bantic force-pushed the bantic/embroider-native-v2-docs-app branch from 3e2e107 to 7017336 Compare January 10, 2025 14:57
@bantic
Copy link
Contributor Author

bantic commented Jan 10, 2025

how we can more consistently leverage Addepar's own linting/style defaults

Yeah -- I don't have a great answer here. I suppose an addepar-specific eslint plugin seems like it would be a good initial step.

@bantic bantic added this pull request to the merge queue Jan 10, 2025
Merged via the queue into master with commit a6df266 Jan 10, 2025
8 checks passed
@bantic bantic deleted the bantic/embroider-native-v2-docs-app branch January 10, 2025 14:59
@github-actions github-actions bot mentioned this pull request Jan 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants