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 .gitattributes file to make line endings more consistent #4679

Merged
merged 2 commits into from Mar 21, 2024

Conversation

aryaemami59
Copy link
Contributor

This PR:

  • Adds .gitattributes file to make line endings more consistent.

Copy link

codesandbox-ci bot commented Feb 21, 2024

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

@timdorr
Copy link
Member

timdorr commented Feb 21, 2024

I would advise against this, for the same reasons that we don't have a formatting check anymore. It becomes a big, annoying barrier to contributions by others who don't set up their tooling the right way before writing some code.

Also, git's autocrlf stuff is godawful. This is basically punishing all Windows devs 😆

@EskiMojo14
Copy link
Contributor

@timdorr doesn't prettier already enforce LF EOL anyway? I'm open to ditching the gitattributes file but the prettierrc we use seems to already have this same opinion

@timdorr
Copy link
Member

timdorr commented Feb 22, 2024

Not in auto mode. It just uses whatever the first line of the file uses.

Honestly, that's fine because nearly any editor outside of Notepad handles differing linebreak styles automatically. It also doesn't matter to end users because everything is run through our tooling, which normalizes everything anyways.

@EskiMojo14
Copy link
Contributor

prettierrc doesn't specify an option, and the default has been LF since Prettier 2

@Methuselah96
Copy link
Member

I develop entirely on Windows and afaik * text=auto eol=lf actually makes it easier for Windows newcomers, especially since Prettier expects LF line endings. It makes it more consistent by cloning using LF line endings.

@timdorr
Copy link
Member

timdorr commented Feb 22, 2024

prettierrc doesn't specify an option, and the default has been LF since Prettier 2

Apologies, I read the PR change backwards. Yeah, we're on LF for folks using Prettier.

Nonetheless, I'm not a huge fan of "big bang", large reformat PRs. I'd just set the setting for changes going forward and leave the existing formatting alone.

@EskiMojo14
Copy link
Contributor

EskiMojo14 commented Feb 27, 2024

@timdorr my concern with that is that you then get spam in unrelated PRs as formatting gets updated 😕
normally the concern with large reformat PRs is causing conflicts in other PRs, but this repo only has 6 other PRs open, half of which are mine and Arya's, and 2 which have already sat long enough to collect conflicts

@timdorr
Copy link
Member

timdorr commented Feb 27, 2024

Well, we have equivalent PRs sitting open in other repos, so it may not be as clean elsewhere. A lot of the changes are in files that aren't touched often, so I wouldn't expect a whole slew of file changes from new PRs after this.

Copy link

netlify bot commented Mar 10, 2024

Deploy Preview for redux-docs ready!

Name Link
🔨 Latest commit bda8a8a
🔍 Latest deploy log https://app.netlify.com/sites/redux-docs/deploys/65fc7446df9bb5000880df09
😎 Deploy Preview https://deploy-preview-4679--redux-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.

@aryaemami59
Copy link
Contributor Author

aryaemami59 commented Mar 17, 2024

@timdorr Would it be easier if I split this into multiple PRs? Or I can Bump Prettier, remove the "trailingComma": "none" and re-format again and then maybe there won't be as many changes.

@timdorr
Copy link
Member

timdorr commented Mar 20, 2024

Let's keep it simple and just make this PR the singular commit of 1d83caa. No need to complicate things by also doing a formatting run. We can make that a separate PR so the noise is kept down.

Basically separate out the policy decision from the effects of that policy. Sort of equivalent to an RFC vs. implementation.

Edit: I just bumped some of these things and formatted them on master, so that's already handled for you.

@aryaemami59
Copy link
Contributor Author

@timdorr Awesome, thanks Tim! I'm gonna do the same with the othe repos.

@aryaemami59 aryaemami59 force-pushed the gitattributes branch 2 times, most recently from ec73f4c to 2e7147e Compare March 21, 2024 17:46
@timdorr
Copy link
Member

timdorr commented Mar 21, 2024

Thanks!

@timdorr timdorr merged commit 81d597f into reduxjs:master Mar 21, 2024
12 checks passed
@aryaemami59 aryaemami59 deleted the gitattributes branch March 21, 2024 18:02
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.

None yet

4 participants