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

Chore/4391/devtooling #2

Merged
merged 30 commits into from
Nov 2, 2023
Merged

Chore/4391/devtooling #2

merged 30 commits into from
Nov 2, 2023

Conversation

pete-watters
Copy link
Contributor

@pete-watters pete-watters commented Oct 25, 2023

This PR adds some changes to the mono-repo:

  • adds a new eslint-config package
  • adds a new tsconfig-config package
  • sets up basic Git actions to run jobs
  • sets up husky and commitlint to enforce standards locally
  • creates and updates documentation

I did not publish the new packages to npm yet. The next step I see is to either:

  • add Nx and setup automatic releases
  • publish manually for now and move panda + design tokens in then the other libs

I found this good example setup I have mostly been taking inspiration from / copying:

He is also using some other things we could consider:

"@changesets/cli": "^2.26.2",
"@commitlint/cli": "^18.0.0",
"@commitlint/config-conventional": "^18.0.0",
"@leather-wallet/eslint-config": "workspace:^",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

workspace:^ means they are installed here from the local workspace

Copy link
Collaborator

Choose a reason for hiding this comment

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

do we want this or workspace:*? Thought I saw that in the nx vids

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You are right, there he declares them as *. I can change it but I'm not 100% what's best to use.

This is the default used by pnpm.

On the pnpm docs they give this example:

Screenshot 2023-11-01 at 08 23 23

On that Nx tutorial they use * but I amn't sure if it gives us benefit
Screenshot 2023-11-01 at 08 22 58

Any ideas what is best for us? 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I changed it to * but that caused the CI jobs to fail with:
Screenshot 2023-11-01 at 08 41 00

Let's keep it as is for now to keep momentum and we can open an issue to change it if it will bring a big benefit?

},
extends: [
'plugin:@typescript-eslint/recommended',
// 'plugin:react/recommended',
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I just commented these out for now. I need to figure out how to extend this file passing in these values in the extension.

I didn't want to add react plugins here as some packages won't need them. Maybe I should?

"noFallthroughCasesInSwitch": true,
"moduleResolution": "node",
"resolveJsonModule": true,
// "baseUrl": "src",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I just commented this for now. I'll remove when integrating to the extension

@markmhendrickson markmhendrickson requested review from fbwoolf and removed request for kyranjamie October 30, 2023 15:51
@kyranjamie kyranjamie requested review from kyranjamie and removed request for fbwoolf October 30, 2023 15:52
Copy link
Contributor

@fbwoolf fbwoolf left a comment

Choose a reason for hiding this comment

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

Looks like a good start @pete-watters 👍

Copy link
Collaborator

@kyranjamie kyranjamie left a comment

Choose a reason for hiding this comment

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

Looks great @pete-watters, awesome start.

Do we need to repeat the LICENSE files?

Comment on lines +1 to +2
engine-strict = true
auto-install-peers = true
Copy link
Collaborator

Choose a reason for hiding this comment

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

needed if we're using pnpm?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not 100% needed - works without so I can remove if you think we don't need to add it?

I just added it based on a monorepo template I was following and it seemed useful

TODO.md Outdated Show resolved Hide resolved
"@changesets/cli": "^2.26.2",
"@commitlint/cli": "^18.0.0",
"@commitlint/config-conventional": "^18.0.0",
"@leather-wallet/eslint-config": "workspace:^",
Copy link
Collaborator

Choose a reason for hiding this comment

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

do we want this or workspace:*? Thought I saw that in the nx vids

.github/workflows/code-checks.yml Outdated Show resolved Hide resolved
@pete-watters
Copy link
Contributor Author

Looks great @pete-watters, awesome start.

Do we need to repeat the LICENSE files?

We don't need to repeat them but I thought if we are publishing packages independently it's nice to have them bundled with all they need, including license.

I can easily remove them though to trim things down

@pete-watters pete-watters merged commit b374422 into dev Nov 2, 2023
4 checks passed
edgarkhanzadian added a commit that referenced this pull request Mar 7, 2024
# This is the 1st commit message:

feat: add web storybook and some design components

# This is the commit message #2:

refactor: move index file to src

# This is the commit message #3:

refactor: expose components through @leather-wallet/design-system/{native|web}

# This is the commit message #4:

refactor: run syncpack format
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.

Setup workflow on monorepo Migrate eslint-config to mono repo Refactor core code to use monorepo packages
3 participants