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

feat: add prettier to project #122

Closed
wants to merge 6 commits into from
Closed

Conversation

ryandotfurrer
Copy link
Member

Added prettier files and created a writeup on installing it from scratch.

We need to make sure we add prettier to the ESLint config file so they do not interfere with one another.

{
  "extends": [... "prettier"]
}

Also updated the README

Copy link

vercel bot commented Apr 10, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
gridiron-survivor ❌ Failed (Inspect) Apr 12, 2024 4:23pm

@ryandotfurrer ryandotfurrer linked an issue Apr 10, 2024 that may be closed by this pull request
Copy link
Contributor

Choose a reason for hiding this comment

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

Image is not viewable in README.md, returns Invalid upstream response (403) when clicking on image link.

Also when I tried to follow the README instructions, I get the following error for the editor.defaultFormater property when trying to put "esbenp.prettier-vscode" as its value: Value is not accepted

  • Tried putting it in dart object
  • Tried putting it in settings.json object

Screenshot 2024-04-10 065831
Screenshot 2024-04-10 065852

Copy link
Contributor

@choir241 choir241 left a comment

Choose a reason for hiding this comment

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

Your PR seems to be failing the Vercel check, please look into this.

@ryandotfurrer please review my comments.

@emestabillo
Copy link
Contributor

@ryandotfurrer Please also switch the base branch to develop

@shashilo shashilo changed the base branch from main to develop April 10, 2024 12:52
Copy link
Collaborator

@shashilo shashilo left a comment

Choose a reason for hiding this comment

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

Everything looks good, expect that broken image.

@chris-nowicki
Copy link
Contributor

Added prettier files and created a writeup on installing it from scratch.

We need to make sure we add prettier to the ESLint config file so they do not interfere with one another.

{
  "extends": [... "prettier"]
}

Also updated the README

question for @ryandotfurrer : Why are we going to lint styling of code? We can either automatically format it on save or by running an npm command. @shashilo what do you think? I am open to both! just never linted prettier before and not sure I understand why we would?

@ryandotfurrer
Copy link
Member Author

question for @ryandotfurrer : Why are we going to lint styling of code? We can either automatically format it on save or by running an npm command. @shashilo what do you think? I am open to both! just never linted prettier before and not sure I understand why we would?

@chris-nowicki to my understanding, this is not linting Prettier, but instead is turning off any ESLint rules that may conflict with our Prettier config and ensures that Prettier's formatting takes precedence over anything ESLint might do.

This allows us to use Prettier to do the code formatting, but allows ESLint to catch code issues. In my opinion, letting each tool do what they do best.

@shashilo, what are your thoughts?

@ryandotfurrer
Copy link
Member Author

@ryandotfurrer Please also switch the base branch to develop

@shashilo did this for me while I was at work. Thank you, Shashi.

@ryandotfurrer
Copy link
Member Author

ryandotfurrer commented Apr 10, 2024

@choir27 Image is not viewable in README.md, returns Invalid upstream response (403) when clicking on image link.

@shashilo Everything looks good, expect that broken image.

It is interesting to me that when I view my fork and look at the README.md the picture is there just fine. View the branch's README and see for yourself.

@ryandotfurrer
Copy link
Member Author

@choir27 Image is not viewable in README.md, returns Invalid upstream response (403) when clicking on image link.

@shashilo Everything looks good, expect that broken image.

It is interesting to me that when I view my fork and look at the README.md the picture is there just fine. View the branch's README and see for yourself.

@chris-nowicki sent me a screenshot of the image not working, so I uploaded the image to a different service and changed the URL. Please let me know if it does not work.

@shashilo
Copy link
Collaborator

Prettier should control the formatting of the coding, while linting catches coding rule errors. We'll have to get them in to see how they work together.

@ryandotfurrer
Copy link
Member Author

Closing to start over using pnpm and see if that resolves my issues with the vercel deployment checks.

@ryandotfurrer ryandotfurrer deleted the feat_rf_add-prettier branch April 12, 2024 17:24
shashilo pushed a commit that referenced this pull request Apr 13, 2024
Closes #122 

Installed Prettier, setup Prettier ignore file (`.prettierignore`),
setup Prettier config file (`pretter.config.js`), installed
`eslint-config-prettier` to make ESLint and Prettier play nice with each
other, and updated the `README`.
Clue355 pushed a commit that referenced this pull request May 1, 2024
Closes #122 

Installed Prettier, setup Prettier ignore file (`.prettierignore`),
setup Prettier config file (`pretter.config.js`), installed
`eslint-config-prettier` to make ESLint and Prettier play nice with each
other, and updated the `README`.
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
5 participants