redux icon indicating copy to clipboard operation
redux copied to clipboard

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

Open aryaemami59 opened this issue 1 year ago • 7 comments

This PR:

  • [X] Adds .gitattributes file to make line endings more consistent.

aryaemami59 avatar Feb 21 '24 21:02 aryaemami59

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.

codesandbox-ci[bot] avatar Feb 21 '24 21:02 codesandbox-ci[bot]

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 😆

timdorr avatar Feb 21 '24 23:02 timdorr

@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

EskiMojo14 avatar Feb 22 '24 00:02 EskiMojo14

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.

timdorr avatar Feb 22 '24 00:02 timdorr

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

EskiMojo14 avatar Feb 22 '24 00:02 EskiMojo14

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.

Methuselah96 avatar Feb 22 '24 02:02 Methuselah96

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.

timdorr avatar Feb 22 '24 04:02 timdorr

@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

EskiMojo14 avatar Feb 27 '24 17:02 EskiMojo14

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.

timdorr avatar Feb 27 '24 17:02 timdorr

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.

netlify[bot] avatar Mar 10 '24 03:03 netlify[bot]

@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.

aryaemami59 avatar Mar 17 '24 09:03 aryaemami59

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.

timdorr avatar Mar 20 '24 16:03 timdorr

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

aryaemami59 avatar Mar 21 '24 16:03 aryaemami59

Thanks!

timdorr avatar Mar 21 '24 18:03 timdorr