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 a .gitattributes file #328

Merged
merged 3 commits into from
Sep 18, 2019
Merged

Add a .gitattributes file #328

merged 3 commits into from
Sep 18, 2019

Conversation

KasperFranz
Copy link
Contributor

This is added to keep the file size when using composer down

This is added to keep the file size when using composer down
@KasperFranz
Copy link
Contributor Author

@rmccue any chance this can get implemented?

@rmccue
Copy link
Collaborator

rmccue commented Sep 16, 2019

@KasperFranz Apologies for the delay here. Happy to merge in; could you merge master into your branch so tests pass please?

@rmccue
Copy link
Collaborator

rmccue commented Sep 16, 2019

@KasperFranz It appears you've rebased here, as the commits now appear as committed by you but authored by someone else. Could you correct this please?

@KasperFranz
Copy link
Contributor Author

It was a quick test, while not being at my machine at home

I will do that later today

@rmccue
Copy link
Collaborator

rmccue commented Sep 16, 2019

No rush, this repo moves fairly slowly :)

@codecov-io
Copy link

codecov-io commented Sep 17, 2019

Codecov Report

Merging #328 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             master     #328   +/-   ##
=========================================
  Coverage     92.11%   92.11%           
  Complexity      760      760           
=========================================
  Files            21       21           
  Lines          1762     1762           
=========================================
  Hits           1623     1623           
  Misses          139      139

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b780740...a53dd92. Read the comment docs.

Copy link
Member

@jrfnl jrfnl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only thing I'm wondering about is the ignore for the docs and the examples.

All the rest isn't needed for someone new to the repo to implement it, but those can be useful for a first time implementation (similar to the readme).

@ozh
Copy link
Collaborator

ozh commented Sep 18, 2019

I'm in favor of ignoring those. I would even remove all non php files, in particular all the .MD files that are much less readable on a server than on Github. The idea is that when using the library as is on a prod server, you don't end up with files that can be used for full path disclosure

@KasperFranz
Copy link
Contributor Author

Only thing I'm wondering about is the ignore for the docs and the examples.

All the rest isn't needed for someone new to the repo to implement it, but those can be useful for a first time implementation (similar to the readme).

My thoughts were that when you are using this (or any other package), I would typically look through the code and the documentation online instead of reading through a docs folder in the vendor directory.

This was partly made to start a discussion about it as well, I am glad to make changes to the code if anyone wants me to change anything.

The branch is also up to date now.

@jrfnl
Copy link
Member

jrfnl commented Sep 18, 2019

I would even remove all non php files, in particular all the .MD files that are much less readable on a server than on Github.

The way the .gitattributes file is now in this PR, it basically does, safe for the cacert.pem file, the readme, changelog and license file and I would never recommend for those to be export-ignored.

Copy link
Collaborator

@rmccue rmccue left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me, I'm 👍 on excluding the docs; agreed on using the website instead.

@rmccue rmccue merged commit 3470169 into WordPress:master Sep 18, 2019
@KasperFranz KasperFranz deleted the patch-1 branch September 18, 2019 10:52
@jrfnl jrfnl added this to the 1.7.1 milestone Sep 23, 2019
@jrfnl jrfnl mentioned this pull request Sep 23, 2019
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.

5 participants