reduxjs / redux

A JS library for predictable global state management
https://redux.js.org
MIT License
60.51k stars 15.26k forks source link

Add `.gitattributes` file to make line endings more consistent #4679

Closed aryaemami59 closed 1 month ago

aryaemami59 commented 2 months ago

This PR:

codesandbox-ci[bot] commented 2 months ago

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 commented 2 months ago

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 commented 2 months ago

@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 commented 2 months ago

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 commented 2 months ago

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

Methuselah96 commented 2 months ago

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 commented 2 months ago

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 commented 2 months ago

@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 commented 2 months ago

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.

netlify[bot] commented 2 months ago

Deploy Preview for redux-docs ready!

Name Link
Latest commit bda8a8ab222f11cca416514870e008864f00298d
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 commented 2 months ago

@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 commented 1 month ago

Let's keep it simple and just make this PR the singular commit of 1d83caa826ecf14ee88e23715c0b74e8bbfbe1f0. 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 commented 1 month ago

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

timdorr commented 1 month ago

Thanks!