Skip to content

Move to Yarn V2 and upgrade packages - #5

Merged
OzzieOrca merged 18 commits into
mainfrom
repo-updates
Feb 12, 2021
Merged

Move to Yarn V2 and upgrade packages#5
OzzieOrca merged 18 commits into
mainfrom
repo-updates

Conversation

@OzzieOrca

@OzzieOrca OzzieOrca commented Feb 12, 2021

Copy link
Copy Markdown
Contributor
  • Migrate to Yarn V2 and Yarn PnP https://yarnpkg.com/features/pnp
  • Include Yarn cache in git repo to enable https://yarnpkg.com/features/zero-installs (Before anyone else gets misled, yes you still have to run yarn install to build packages that have postinstall scripts that get placed in the .yarn/unplugged directory and aren't committed to the repo since they can be machine dependant. But most other packages can just be grabbed from the repo cache.)
  • Add peer dependencies to package.json since Yarn V2 is more strict
  • Add @next/bundle-analyzer
  • Use tsc for linting and babel for compiling
  • Migrate images to use import instead of require
  • Fix tests where a mock was referencing an out of scope variable

@OzzieOrca
OzzieOrca force-pushed the repo-updates branch 2 times, most recently from d35e13b to 582b587 Compare February 12, 2021 02:16
@vercel

vercel Bot commented Feb 12, 2021

Copy link
Copy Markdown

Deployment failed with the following error:

Creating the Deployment Timed Out.

@vercel

vercel Bot commented Feb 12, 2021

Copy link
Copy Markdown

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/cru/mpdx-react/3slvtkzol
✅ Preview: https://mpdx-react-git-repo-updates.cru.vercel.app

@@ -32,7 +32,7 @@ describe('TaskDrawerForm', () => {
await waitFor(() => expect(getByText('Save')).not.toBeDisabled());
userEvent.click(getByText('Save'));
await waitFor(() => expect(onClose).toHaveBeenCalled());
});
}, 10000);

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

These tests still have issues but increasing the Jest timeout on individual tests is better than increasing it globally.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Agreed 👍

@TheNoodleMoose TheNoodleMoose left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks pretty good, excited to give yarn v2 a try!

@@ -7,6 +7,7 @@ import { cloneDeep } from 'lodash/fp';
import { useApp } from '../../../../App';
import { GetNotificationsQuery } from '../../../../../../types/GetNotificationsQuery';
import { AcknowledgeAllUserNotificationsMutation } from '../../../../../../types/AcknowledgeAllUserNotificationsMutation';
import illustration13 from '../../../../../images/drawkit/grape/drawkit-grape-pack-illustration-13.svg';

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Do you think these should eventually be more declarative names or do you think it's fine as is?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

They are from https://www.drawkit.io/product/grape-illustration-pack and the filenames are just numbers. I could try to describe them but they are all vague and it didn't seem necessary right now. Maybe we can label them better in the future... Idk what direction design is going to want to go with them.

@@ -32,7 +32,7 @@ describe('TaskDrawerForm', () => {
await waitFor(() => expect(getByText('Save')).not.toBeDisabled());
userEvent.click(getByText('Save'));
await waitFor(() => expect(onClose).toHaveBeenCalled());
});
}, 10000);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Agreed 👍

Comment thread .vscode/extensions.json
@@ -0,0 +1,7 @@
{
"recommendations": [

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Will this do anything if you don't have the extensions installed?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It shows a popup suggesting you install the extension. I might add a Jest one. Feel free to add extensions but they should probably be tooling specific instead of something like exploding rainbows as you type haha.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

But that's the best one! 🤣

@OzzieOrca
OzzieOrca merged commit 2ac0a3b into main Feb 12, 2021
@OzzieOrca
OzzieOrca deleted the repo-updates branch February 12, 2021 20:02
dr-bizz added a commit that referenced this pull request Jun 25, 2026
- #23 config-driven import graph: resolveImport/buildGraph/indexStore take aliases/
  extensions/roots from config.index (defaults preserved); schema + tests
- #4 single arg parser (engine/args.cjs) used by plan/impact/indexStore/learningsStore
  (no more 4 divergent parsers); review:* scripts repointed to the cli.cjs dispatcher;
  cli index --force
- #5 command gates index/learning via 'review config get', not raw grep of config.yml
- #1 auto-fix apply_all.sh now DRY-RUNS untrusted model-generated scripts; applies only
  with --yes
- 68 tests pass
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants