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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a .gitattributes files #1559

Merged
merged 2 commits into from Aug 18, 2019
Merged

Add a .gitattributes files #1559

merged 2 commits into from Aug 18, 2019

Conversation

ericcornelissen
Copy link
Contributor

Issue: Related to #1427

Description

The Composer/Packagist issue convinced me to implement this (though it turned out to not be strictly necessary from an export-ignore point of view), but I have been thinking about adding a .gitattributes file to the project for a long time. The primary things it will provide us with is better diffing and more consistency across platforms (e.g. line-endings).

The contents of the file are obviously open for discussion 馃檪

@ericcornelissen ericcornelissen added the meta Issues or pull requests regarding the project or repository itself label Jul 26, 2019
@davidklebanoff davidklebanoff self-requested a review August 5, 2019 21:26
@birjj
Copy link
Contributor

birjj commented Aug 15, 2019

My only question is what impact the export-ignore lines have on our project. Does NPM or GitHub use the git archive-generated export?

Everything else looks good to me (and a welcome configuration change). Thanks for handling it! Also sorry for the super late review.

@ericcornelissen
Copy link
Contributor Author

My only question is what impact the export-ignore lines have on our project. Does NPM or GitHub use the git archive-generated export?

On GitHub it affects the content so the zip file you get when you download the repository as zip (repository homepage > "Clone or download" > "Download ZIP").

As far as I know(/can tell) it doesn't affect NPM releases and running npm publish just looks at the files in the directory, ignoring only files specified in .npmignore (or .gitignore);

Also sorry for the super late review.

Not a problem 馃槈

@birjj
Copy link
Contributor

birjj commented Aug 15, 2019

I think that in that case I'd prefer not excluding files from it. When clicking the download button I would expect to get a ZIP of all the files I see in the web interface too.

@ericcornelissen
Copy link
Contributor Author

I think that in that case I'd prefer not excluding files from it. When clicking the download button I would expect to get a ZIP of all the files I see in the web interface too.

I can agree with that to some extend. To clarify, the export-ignore is intended to be used to manage what is included in the archive when you run the git archive command. The fact that GitHub uses that to generate the zip file may lead to awkward situation when a user expects that the download the whole repository. However, I think that is primarily GitHub's issue, not ours.

Regardless, I do agree that if you use it for that purpose the number of files ignored in the export can be far less. For now I have reduced it to:

  • CNAME: only relevant for our GitHub pages site.
  • .github (folder): is only relevant to GitHub, not the project itself.
  • .gitpod.yml: is only relevant on GitHub, not to the project itself.
  • .travis.yml: is only relevant on GitHub, not to the project itself.

@davidklebanoff davidklebanoff removed their request for review August 15, 2019 17:56
@birjj birjj merged commit 9d5efeb into simple-icons:develop Aug 18, 2019
@ericcornelissen ericcornelissen deleted the git/gitattributes branch August 18, 2019 10:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
meta Issues or pull requests regarding the project or repository itself
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants