chore: configure prettier pre-commit step and autoformat entire repo#300
Conversation
✅ Deploy Preview for compilerla ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
ea08120 to
d639b03
Compare
Scotchester
left a comment
There was a problem hiding this comment.
A few comments to get us started.
d639b03 to
17b1cfc
Compare
Scotchester
left a comment
There was a problem hiding this comment.
i'd like to wait until we're reasonably certain we don't want to change any other prettier/shopify defaults before i rewrite history
I'll preface this by saying, if this is the same config you just implemented on calitp.org, I am totally fine to roll with it, but if we're opening the discussion… skimming through the formatting commit's diff, there are a few things that stand out to me:
- This kind of breaking of tags across different lines remains intensely annoying, but IIRC we couldn't find a reasonable way to avoid it.
<span class="secondary-sans-serif-xs d-inline-block pb-4 mb-3" >An inside look at Compiler’s internal projects, interests and our client projects.</span >
- Indendation inside
{% comment %}blocks. I guess NBD if the comment toggle shortcut in VS Code handles the indentation/deindentation automatically. - I prefer having the first attribute of an HTML tag on the same line as the tag name and the subsequent attributes lined up with it on the lines below.
- I don't like that it's adding a
/before the closing>of self-closing tags. - I like the (old school?) convention of not indenting an HTML doc's
headandbodytags, but this is the least offensive thing on this list to me.
if anything in that tick list can be mitigated/improved by altering prettier or shopify's defaults, I'm not opposed. with regard to the tag wrapping, I too recollect that we tried and couldn't find a suitable alternative, but it's not outside the realm of possibility that there's something we missed. I have to admit that my personal enthusiasm for optimization is low. I'm happy to just accept prettier's defaults, but if anyone else wants to play around with this branch before the PR is merged or create another branch of their own later, all the pieces are present to automate reformatting the entire repo and ensure that the diff is ignored afterward. |
|
Seems like a decent way to spend the last couple hours of the ride into Chicago :) As I brought up the docs to dig into what options were available, I noticed this in the VS Code extension's README:
I'll see if I can test this out. |
I haven't completely confirmed this yet, but I DID run across two other things I want to mention quickly:
Maybe this is referring to the same root issue as the Prettier extension README, but it seems like whatever the case, we'll want to list each language separately. |
in the wild so far we've only seen a collision for json (you) and html (@angela-tran). personally i'm fine with limiting our overrides to what we've actually seen, but if you prefer to be exhaustive, i don't have a problem with that either. |
all good @indexing! @compilerla/compiler-comms is assigned by default in this repo anytime a change is made in the _posts directory with the assumption being that we're usually either publishing a new blog or making a copy change. compiler.la/.github/CODEOWNERS Line 3 in dafad11 if you ever see a PR like this that isn't a clear attempt to update the content on the https://compiler.la website, i'd say its safe to ignore. you're also always welcome to ask for clarification before you spend time on a fullblown code review. |
|
I just did some more testing with vscode-settings-inspector to confirm that listing them individually works and listing them together does not.
It's just such a hidden footgun, and hard to realize exactly what's happening when it goes off, I would prefer to be exhaustive, yes. I could push that change to this branch, if you're cool with that. |
of course. you have my blessing! 📿 |
I did some testing and can confirm too that we need to list them individually. Thanks for catching @Scotchester! |
angela-tran
left a comment
There was a problem hiding this comment.
This looks good to me 👍
I'm good with sticking with prettier's defaults. We can always revisit it, and I think getting through the 11ty conversion work is the higher priority.
|
Having satiated my curiosity about the process of updating Prettier in the future, I concur with Angela, let's merge this and move forward. |
closes #296
personally i'm fine with prettier and the shopify plugin's defaults, but @angela-tran, feel free to play around and test the effect of making changes to the options for quotes (or anything else) if you'd like.