config: sync local/CI Prettier and Restyled

[jorgeorpinel]

for #2641

Should close #2641

[jorgeorpinel]

☝️ Local format checks passed for the above commit (good change):

CI checks and Restyled also passed:

[jorgeorpinel]

☝️ I skipped local tests with the above commit (git commit -n) since I knew the formatting is wrong.

The CI tests failed but Restyled (which took a while BTW) is passing 🤔

[jorgeorpinel]

☝️ For some reason reverting #2624 (review) makes Restyled work again. Maybe it can't process that many files? Cc @casperdcl

[jorgeorpinel]

Please review but don't merge (since checks are failing) @shcheklein. We need to revert the changes to content/docs/user-guide/contributing/blog.md once this is approved. Thanks

[shcheklein]

Thanks @jorgeorpinel . What was the reason for the discrepancy?

[jorgeorpinel]

I don't think it's a discrepancy @shcheklein. My hypothesis is that Restyled isn't able to process that many files (see #2624 (review)). TBH I'm not sure what the problem is here but for now this seems to help.

If there's a discrepancy then there are 2 problems. I didn't investigate that aspect much ⏳

[shcheklein]

Is there a link to logs / Restyled PR that we think it did wrong?

[jorgeorpinel]

If there's a discrepancy

I sorted the Prettier settings in both config files in 36ae1cf to compare them easily and they seem to be the same. There could be a Prettier version discrepancy I guess but I doubt it or that it would cause a problem.

Is there a link to logs / Restyled PR that we think it did wrong?

There's https://restyled.io/gh/iterative/repos/dvc.org/jobs, let me see if I can find something in there... ⏳

[jorgeorpinel]

The logs are not super informative. They only output when it decided to format files, but doesn't show the output of Prettier itself.

Examlpe: https://restyled.io/gh/iterative/repos/dvc.org/jobs/1096523

[shcheklein]

so, it does change blog.md .. but that's expected, right?

[jorgeorpinel]

it does change blog.md

@shcheklein yeah sorry for the confusion. That log is from the current config (where it works as intended). For the previous config (when Restyled always passed) it doesn't say anything useful:

Example: https://restyled.io/gh/iterative/repos/dvc.org/jobs/1096496

[casperdcl]

note there may also be issues with eslint (iterative/setup-dvc#22)

For me, doing this:

• current master (7139bde)
• node v14.17.4
• yarn && npm install as recommended in https://dvc.org/doc/user-guide/contributing/docs#development-environment

then

• yarn lint-ts: finds 5 errors
• yarn lint-css: succeeds
• yarn format-all: 10 files changed, 57 insertions(+), 60 deletions(-)
• npm update: updates 10 package versions in package.json

Opened a PR at #2779 for the above. Not sure if it's related to this (#2774) PR.

[shcheklein]

Let's try to get run if { } in the config, but keep all the patterns:

    include: 
      - "**/*.md"
      - "**/*.jsx"
      - "**/*.tsx"
      - "**/*.js"
      - "**/*.ts"
      - "**/*.json"

[shcheklein]

I would also create a ticket/ask in the resyled repo- it's open source, they can help.

[jorgeorpinel]

to make it consistent with the package.json settings

Right, the check does format-check indeed. I think originally Restyled intended to replace the local Git hook instead though, for people who don't install yarn locally (e.g. edit via Github) and that only runs format-staged. But anyway, it should be consistent with the GH action check indeed, I suppose.

Let's try to get run if { } in the config, but keep all the patterns

Done in 26fc2e3 (after master merge). Seems to be working for now 🤔 (see Restyled run).

[jorgeorpinel]

Seems to be working

I retested the previous config file and Restyled stopped failing again (unwanted behavior). So I guess it was just a config file formatting issue after all. Good catch @shcheklein ! I force pushed 26fc2e3 again now and it looks good:

I'm going to merge #2775 to fix the formatting here. Everything should pass after that. Please approve/merge 🙂

[jorgeorpinel]

Wait I still need to roll back the dummy content changes. I'll merge it...

[jorgeorpinel]

Shoot so now Restyled removed this line but prettier adds it back, so the check is failing... Looks like there's still a config discrepancy somewhere 😕

[jorgeorpinel]

Hmmm I reintroduced the line in a9b208e and Restyled left it alone this time 🤷

Some more research is still needed.

[jorgeorpinel]

OK everything is actually passing now. I think we can reapprove and merge this now @shcheklein. Sorry for the confusion. I'll keep an eye on it... 👀

[shcheklein]

I would put a comment here, btw

[jorgeorpinel]

Done in f61f8be.