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

Format files #998

Closed
wants to merge 3 commits into from
Closed

Format files #998

wants to merge 3 commits into from

Conversation

amirhhashemi
Copy link
Contributor

@amirhhashemi amirhhashemi commented Jan 2, 2025

  • I have read the Contribution guide
  • This PR references an issue (except for typos, broken links, or other minor problems)

Description(required)

There are many unformatted files in this repository. I often encounter these when I'm editing a file, and my editor automatically formats the file on save. This can lead to numerous unintended changes, as those files were not formatted beforehand. As a result, I usually have to manually revert these changes to keep the PR clean.

To fix it, first I ran pnpm prettier --write . to format all files (a2e1a39). Then I fixed a few errors manually (92cc86c)

To prevent unformatted files from being merged, it's also essential to format them in the CI. I can implement this if you’re interested.

#999 fixes the Eslint error.

Copy link

stackblitz bot commented Jan 2, 2025

Review PR in StackBlitz Codeflow Run & review this pull request in StackBlitz Codeflow.

Copy link

netlify bot commented Jan 2, 2025

Deploy Preview for solid-docs ready!

Name Link
🔨 Latest commit 3c790e3
🔍 Latest deploy log https://app.netlify.com/sites/solid-docs/deploys/6776c3e70d5b8a0008c64416
😎 Deploy Preview https://deploy-preview-998--solid-docs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@amirhhashemi amirhhashemi marked this pull request as draft January 2, 2025 16:18
@amirhhashemi amirhhashemi marked this pull request as ready for review January 2, 2025 16:51
@LadyBluenotes
Copy link
Member

Thanks for taking this on @amirhhashemi. I think i'm going to close this issue and have someone from the docs team tackle it.

Do you mind making an issue for this, instead?

@amirhhashemi
Copy link
Contributor Author

Thanks for taking this on @amirhhashemi. I think i'm going to close this issue and have someone from the docs team tackle it.

Do you mind making an issue for this, instead?

But why? It's just Prettier. Ready for review.

@LadyBluenotes
Copy link
Member

LadyBluenotes commented Jan 2, 2025

Anything that impacts configuration should be handled by the team unless they are small fixes. It might be worthwhile reviewing the Contribution Guide as we go over how we like to approach issues and PRs.

For one, there are aspects of the docs that can be affected by these types of changes. I would rather have team members who are familiar with the site infra itself address them vs contributors to avoid it breaking parts of it. Since this would effect thousands of lines, I'd feel more comfortable having a team member address it.

We greatly appreciate each and every one of our contributors and the work they put into PRs, but we do need to follow the procedures outlined in the Contribution Guide. This includes having issues filed and approved prior to having a PR, to start, just to make sure it aligns with our goals for the project and doesn't break anything in the process.

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.

2 participants