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

Add prettier/editorconfigs from 11ty/eleventy, format files #1651

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

uncenter
Copy link
Contributor

@uncenter uncenter commented Jan 6, 2024

Closes #1610.

@uncenter uncenter changed the title Add prettier/editorconfigs from 11ty/eleventy, update prettier Add prettier/editorconfigs from 11ty/eleventy, format files Jan 6, 2024
@uncenter
Copy link
Contributor Author

I won't format the files in this PR to avoid conflicts, but we should run the command at some point...

Copy link
Member

@Snapstromegon Snapstromegon left a comment

Choose a reason for hiding this comment

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

Hi,
I'm all pro adding a formatter. Did you try running this and see if it has problems with the autogenerated files?
Also do you know whether or not this is run on any git hooks (this might lead to issues until all files in the repo are formatted)?

.prettierrc.json Outdated Show resolved Hide resolved
@uncenter
Copy link
Contributor Author

uncenter commented Jan 11, 2024

Also do you know whether or not this is run on any git hooks (this might lead to issues until all files in the repo are formatted)?

I don't really understand or use Git hooks but it looks to be the case:

  "lint-staged": {
    "*.{js,css,md}": [
      "prettier --write",
      "git add"
    ]
  },

I chose to avoid formatting it here because a) the diff is huge, +-30k lines, and b) I don't want to block other PRs.

Did you try running this and see if it has problems with the autogenerated files?

What do you mean by autogenerated files? If there are any we can just add them to a .prettierignore. There are some files that Prettier struggles with, quite understandably though:

Screenshot 2024-01-11 at 10 02 40 (Visual Studio Code)

That seems like an interesting approach to whatever that is!

@Snapstromegon
Copy link
Member

What do you mean by autogenerated files?

The files in https://github.com/11ty/11ty-website/tree/main/src/_data/community are generated by a GH action from the "Build With 11ty" issues.

@uncenter
Copy link
Contributor Author

uncenter commented Jan 11, 2024

Ah, strange. Well, we can just run the format command after that step in the workflow.

@uncenter
Copy link
Contributor Author

...where is that workflow?

@uncenter
Copy link
Contributor Author

I think you are confusing this repo with https://github.com/11ty/11ty-community/tree/main/built-with-eleventy maybe?

@Snapstromegon
Copy link
Member

I think you are confusing this repo with https://github.com/11ty/11ty-community/tree/main/built-with-eleventy maybe?

Ah yes, my mistake

@uncenter
Copy link
Contributor Author

Is there anything else I need to do for this PR? I've ignored that one directory I mentioned that Prettier didn't like, so the command passes now.

Copy link
Member

@Snapstromegon Snapstromegon left a comment

Choose a reason for hiding this comment

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

LGTM, but since this is a fairly fundamental change how this repo is worked on, I'd like the opinion of @zachleat on this.

@uncenter
Copy link
Contributor Author

FYI, the formatting config files (.prettierrc.json, .editorconfig) that I've included here are directly copied from the core @11ty/eleventy repository. So there isn't anything Zach hasn't already approved, technically.

@Snapstromegon
Copy link
Member

I think that these configs have to be seen in the context of a project/repo. E.g. editorconfig settings can vary depending on the project. I do not expect Zach to oppose this change, but it makes me more comfortable to have his approval for now.

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.

Add prettier config
2 participants