#671: configured prettier for new developers #683

River-unknown · 2024-05-05T10:01:43Z

Short Description

Worked on the issue for ensuring prettier gets configured.\

Changes done to the Readme file
Changes done to the package.json ensuring pre-commit hooks

Issues

Closes #671

josephjclark

Hi @River-unknown! Thank you so much for this contribution.

I am not a fan of the pre-commit hook approach. Few things in this world scare me as much as the idea of my code changing invisibly right before I commit it.

I would prefer to rely on editor integrations alongside something like a test:lint or test:format script in package.json which can be run locally or in CI.

I'd also like the readme to be minimally affected by this stuff. We don't need lots of instructions - just a mention of the plugin should be fine.

…file

…be run locally or in CI

.github/workflows/ci.yaml

josephjclark

Hi @River-unknown - thanks again for the changes. I've taken another look and I've got a couple more comments for you.

josephjclark · 2024-05-07T14:36:29Z

README.md

+You can also manually format files using Prettier by running the following command:
+
+```bash
+npm run format


In this repo we usually use pnpm, so this needs to be pnpm run format

package.json

josephjclark · 2024-05-07T14:40:15Z

package.json

@@ -28,7 +32,9 @@
    "@types/gunzip-maybe": "^1.4.0",
    "@types/rimraf": "^3.0.2",
    "@types/tar-stream": "^2.2.2",
+    "eslint": "^8.0.0",


Please remove eslint and the lint commands - between TypeScript's compiler and prettier's formatter, we should be well covered. Thanks!

README.md

josephjclark · 2024-05-13T14:00:43Z

Hi @River-unknown - I'd love to get this work finished off! Can you spare a bit more time to make the final changes?

River-unknown · 2024-05-13T14:06:23Z

Sure sir... I will complete it in 2-3 days..

josephjclark · 2024-05-28T10:02:04Z

Hi @River-unknown, if you're not able to complete this work soon I will need to close this PR and open the issue to other new contributors.

I don't think there's a lot of work outstanding.

If you are having difficulties with any particular point please let me know and we can discuss

River-unknown · 2024-05-28T10:14:35Z

Ya working on it. I will be by tomorrow push certain changes about the issue.

Resolved conflicts

River-unknown · 2024-05-28T13:25:35Z

having issue with resolving confliicts in the pnpm-lock.yaml and am not sure how to resolve that.

josephjclark · 2024-05-28T15:12:10Z

HI @River-unknown you need to merge the main branch into yours and run pnpm install . That will sort out the lock file. You can then commit the changes and push them

josephjclark · 2024-05-28T15:14:42Z

package.json

@@ -28,7 +30,7 @@
    "@types/gunzip-maybe": "^1.4.0",
    "@types/rimraf": "^3.0.2",
    "@types/tar-stream": "^2.2.2",
-    "gunzip-maybe": "^1.4.2",


I'm not sure what happened here but we still need this dependency!

josephjclark · 2024-05-28T16:50:49Z

If I run:

pnpm test:format

From root, I get this error:

[error] No files matching the pattern were found: "src/**/*.{ts,tsx,js,jsx}".

I also have diffs in my pnpm-lock.yaml file after I run pnpm install , which is a bit strange 🤔

EDIT: to be clear it's correct to check the src dir, but I think you want this pattern:

"packages/*/src/**/*.{ts,tsx}"

josephjclark · 2024-05-28T16:58:05Z

package.json

@@ -28,7 +31,7 @@
    "@types/gunzip-maybe": "^1.4.0",
    "@types/rimraf": "^3.0.2",
    "@types/tar-stream": "^2.2.2",
-    "gunzip-maybe": "^1.4.2",
+    "prettier": "^3.2.5",


Can we lock the prettier version to 2.8.8?

It's a bit silly but that's what the repo currently uses. If we use 3.0 a lot of defaults change and we suddenly start seeing an awful lot of changes

I don't want to go through and check all those reformatting diffs. But if we lock it to 2.8.8 we should get a much smaller number of changes, and I can open a new issue to update to pretter 3.x

I've just run prettier 2.8.8 against main and merged the diffs.

So on your branch right now, pnpm test:format should FAIL because it's detecting formatting changes.

After you merge main again, it should pass, and pnpm prettier should generate no diffs

River-unknown · 2024-05-28T16:58:18Z

I just merged main branch into my branch as you said and it happened to be like this. And am not quite sure of that pnpm-lock.ya file matter and for that test i think by mistake i had deleted something i am not sure. I will check back on and will update soon

josephjclark · 2024-05-28T17:03:18Z

Ah @River-unknown I see the problem - you're on a more advanced pnpm version that we are.

I guess we ought to update that soon for now, but for now could you please set your pnpm to version 8.x and re-install from there?

The problem is that your pnpm has updated the lockfile version and changed everything.

River-unknown · 2024-05-28T17:18:06Z

Ahh ok @josephjclark will update to those versions.

River-unknown · 2024-05-28T17:35:55Z

@josephjclark can you please review . I had locked the prettier version to 2.8.8 and also reinstalled pnpm and its dependency with its version 8.

josephjclark · 2024-05-28T17:53:17Z

Great @River-unknown - thank you for all your hard work 🙏

I'll merge this into our repo now, run some final tests, and merge to main in the morning

OpenFn#671: configured prettier

ac2f895

River-unknown marked this pull request as draft May 5, 2024 10:02

River-unknown marked this pull request as ready for review May 5, 2024 10:04

josephjclark requested changes May 7, 2024

View reviewed changes

River-unknown added 2 commits May 7, 2024 15:25

Removed the pre-hook commit section and also minimized the Readme.md …

d3b1894

…file

also added test:lint or test:format script in package.json which can …

1779ad8

…be run locally or in CI

River-unknown requested a review from josephjclark May 7, 2024 10:05

josephjclark reviewed May 7, 2024

View reviewed changes

.github/workflows/ci.yaml Outdated Show resolved Hide resolved

Delete ci.yaml

7f94675

River-unknown requested a review from josephjclark May 7, 2024 10:12

josephjclark requested changes May 7, 2024

View reviewed changes

River-unknown added 3 commits May 28, 2024 18:38

Updated changes

e7f9c5e

Update pnpm-lock.yaml

0579ccb

Resolved conflicts

Update pnpm-lock.yaml

7f9c620

River-unknown requested a review from josephjclark May 28, 2024 13:25

josephjclark reviewed May 28, 2024

View reviewed changes

River-unknown added 2 commits May 28, 2024 21:50

Update package.json

2e8c0b6

Update pnpm-lock.yaml

5ae1659

River-unknown requested a review from josephjclark May 28, 2024 16:24

River-unknown added 2 commits May 28, 2024 21:57

Merge remote-tracking branch 'upstream/main'

5d97054

Update pnpm-lock.yaml

e75bb21

josephjclark reviewed May 28, 2024

View reviewed changes

River-unknown added 2 commits May 28, 2024 22:51

Merge remote-tracking branch 'upstream/main'

44b93a3

changed the versions for consistency

7111126

River-unknown requested a review from josephjclark May 28, 2024 17:36

josephjclark changed the base branch from main to prettier-tooling May 28, 2024 17:54

josephjclark merged commit eb5a39e into OpenFn:prettier-tooling May 28, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

#671: configured prettier for new developers #683

#671: configured prettier for new developers #683

River-unknown commented May 5, 2024 •

edited by josephjclark

josephjclark left a comment

josephjclark left a comment

josephjclark May 7, 2024

josephjclark May 7, 2024

josephjclark commented May 13, 2024

River-unknown commented May 13, 2024

josephjclark commented May 28, 2024

River-unknown commented May 28, 2024 •

edited

River-unknown commented May 28, 2024

josephjclark commented May 28, 2024

josephjclark May 28, 2024

josephjclark commented May 28, 2024 •

edited

josephjclark May 28, 2024

josephjclark May 28, 2024

River-unknown commented May 28, 2024

josephjclark commented May 28, 2024

River-unknown commented May 28, 2024

River-unknown commented May 28, 2024

josephjclark commented May 28, 2024

#671: configured prettier for new developers #683

#671: configured prettier for new developers #683

Conversation

River-unknown commented May 5, 2024 • edited by josephjclark

Short Description

Issues

josephjclark left a comment

Choose a reason for hiding this comment

josephjclark left a comment

Choose a reason for hiding this comment

josephjclark May 7, 2024

Choose a reason for hiding this comment

josephjclark May 7, 2024

Choose a reason for hiding this comment

josephjclark commented May 13, 2024

River-unknown commented May 13, 2024

josephjclark commented May 28, 2024

River-unknown commented May 28, 2024 • edited

River-unknown commented May 28, 2024

josephjclark commented May 28, 2024

josephjclark May 28, 2024

Choose a reason for hiding this comment

josephjclark commented May 28, 2024 • edited

josephjclark May 28, 2024

Choose a reason for hiding this comment

josephjclark May 28, 2024

Choose a reason for hiding this comment

River-unknown commented May 28, 2024

josephjclark commented May 28, 2024

River-unknown commented May 28, 2024

River-unknown commented May 28, 2024

josephjclark commented May 28, 2024

River-unknown commented May 5, 2024 •

edited by josephjclark

River-unknown commented May 28, 2024 •

edited

josephjclark commented May 28, 2024 •

edited