Replies: 17 comments
-
This will be looked at further in break out rooms. |
Beta Was this translation helpful? Give feedback.
-
@drubgrubby and I are going to meet this week to go over this issue |
Beta Was this translation helpful? Give feedback.
-
Good source from @dmcneary https://mskelton.medium.com/auto-formatting-code-using-prettier-and-github-actions-ed458f58b7df |
Beta Was this translation helpful? Give feedback.
-
@drubgrubby @ahdithebomb @martham0 @dmcneary @hackforla/owners @ExperimentsInHonesty @pawan92 This is a demo repo with GitHub action configured to run prettier on all pull request. I believe you can check it out in action by creating a pull request with weirdly formatted js,md and json files . Blockers
Resources/InstructionsCreate a GitHub action with the code snippet below
|
Beta Was this translation helpful? Give feedback.
-
@cnk - We are looking at implementing something to clean up and standardize code being submitted. We have started down a path using Prettier, which seems promising and @akibrhast has done some great testing and research. In your advisory capacity, can you suggest how best to proceed in implementing something like this? It seems like it could be a great benefit, but I can see how it might cause some problems if not implemented carefully. Being the careful one, I want to make sure we're doing our due diligence. Do you have any thoughts on this? Thanks. |
Beta Was this translation helpful? Give feedback.
-
I have some serious reservations about automating changes to our html/md files because they contain a mix of html, javascript, and Liquid template tags. We have already released a couple of bugs because someone's code editor "fixed" some things it did not understand. Do you have a link to anything showing that prettier understands Liquid? Something like https://prettier.io/docs/en/precommit.html would be a bit safer - but then everyone would have to have node installed to work on the project. |
Beta Was this translation helpful? Give feedback.
-
@akibrhast - (From cynthia) "Do you have a link to anything showing that prettier understands Liquid?" |
Beta Was this translation helpful? Give feedback.
-
@cnk @drubgrubby |
Beta Was this translation helpful? Give feedback.
-
@ExperimentsInHonesty - Not sure what the next step in decision making on this is. Should we put in questions and discuss on Sunday? |
Beta Was this translation helpful? Give feedback.
-
I don't remember if anyone mentioned this, but I found another option that I thought may be easier and give us more control. We can make our own ".prettierrc" file and configure the formatting so everyone has the same prettier conventions.
|
Beta Was this translation helpful? Give feedback.
-
As @cnk mentioned, using Prettier locally would be an option but would force each dev to use Node, as the package is installed with npm. The advantage to running the formatter as a Github Action would be that the dependencies for the website as a project stay close to where they are now. If anyone feels up to investigating more, I would suggest running a forked repo of the website through the Prettier GH Action, then seeing what/if anything breaks. I'm pretty sure that it can handle Markdown files and Liquid in HTML by now, though those formats were unsupported before. |
Beta Was this translation helpful? Give feedback.
-
Great research! I think the way forward is:
|
Beta Was this translation helpful? Give feedback.
-
I would add to what CNK said above that we should add something to our readme (which will soon be the contributing file) about installing the prettier extension on your ide to catch formatting errors before you submit your pr. Isn't there a github install for a CI that uses prettier? |
Beta Was this translation helpful? Give feedback.
-
We might also like to look into ignoring our giant cleanup commit when we do |
Beta Was this translation helpful? Give feedback.
-
Does anyone have any progress on this? Maybe we should discuss this in the next meeting |
Beta Was this translation helpful? Give feedback.
-
These is an image from a recent pull request I looked at. You will notice the diff is just the ide auto formatting spaces. I believe a possible solution to this would be add a // The default end of line character.
// - \n: LF
// - \r\n: CRLF
// - auto: Uses operating system specific end of line character.
"files.eol": "\n"
// Format a file on save. A formatter must be available, the file must not be saved after delay, and the
// editor must not be shutting down.
"editor.formatOnSave": false, |
Beta Was this translation helpful? Give feedback.
-
Overview
In the dev meeting @trtincher and @martham0 noticed that our code extensions were changing the code in our pull requests unintentionally. We need to come up with a solution to standardize this.
Action Items
Resources/Instructions
Possible solutions
Beta Was this translation helpful? Give feedback.
All reactions