Skip to content

Fix #649 - Remove useless files from users and servers#679

Merged
Nek- merged 3 commits intoKnpLabs:masterfrom
williamdes:gitattrs
Nov 3, 2022
Merged

Fix #649 - Remove useless files from users and servers#679
Nek- merged 3 commits intoKnpLabs:masterfrom
williamdes:gitattrs

Conversation

@williamdes
Copy link
Copy Markdown
Contributor

@williamdes williamdes commented Oct 10, 2022 •
edited
Loading

Hi !
I did this change in two steps, first I excluded the files from the git archive: git archive --help. GitHub uses git archive for users to be able to "Download" a branch or git tag.

So I kept the test files and the phpunit config in it because: packing people on Debian/Alpine/Others... will download the source using scripts and do not want to have to clone your git history because they do not care about it and only want the source.
That said this is not what composer users want. They only want source file, so I excluded test files from the bundles that packagist produces.

Final users will only have source files, packaging people will have source+tests and devs can use git clone.

# Download source from my PR branch & extract
# composer install
~/Gaufrette-gitattrs$ ./vendor/bin/phpunit 
PHPUnit 8.5.30

SSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSS  63 / 207 ( 30%)
SSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSS........ 126 / 207 ( 60%)
.........SSSSSSSSSSS........................................... 189 / 207 ( 91%)
..................                                              207 / 207 (100%)

Time: 131 ms, Memory: 12.00 MB

OK, but incomplete, skipped, or risky tests!
Tests: 207, Assertions: 170, Skipped: 129.

Fixes: #649

@williamdes williamdes changed the title Gitattrs Fix #649 - Remove useless files from users and servers Oct 10, 2022
Nek-
Nek- suggested changes Oct 12, 2022
View reviewed changes
Copy link
Copy Markdown
Contributor

@Nek- Nek- left a comment

Choose a reason for hiding this comment

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

Thank you for your help on improving Gaufrette. This could be part of the next release (coming soon) but I think you may have excluded too much files if I follow the logic of your PR description.

composer.json Outdated
"archive": {
"exclude": [
"/tests",
"/phpunit.xml.dist"
Copy link
Copy Markdown
Contributor

@Nek- Nek- Oct 12, 2022

Choose a reason for hiding this comment

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

spec folder and phpspec config file should be here to follow the logic entirely (it's part of the test suite). The .env.dist file is also absolutely required, as well as the make file.

@williamdes williamdes requested a review from Nek- October 14, 2022 19:07
@williamdes
Copy link
Copy Markdown
Contributor Author

williamdes commented Oct 14, 2022

Thank you for your help on improving Gaufrette. This could be part of the next release (coming soon) but I think you may have excluded too much files if I follow the logic of your PR description.

Very cool !
I figured out that the Makefile without the docker files would not make sense so I also moved it for them. Should be okay now

@Nek-
Copy link
Copy Markdown
Contributor

Nek- commented Oct 15, 2022

I figured out that the Makefile without the docker files would not make sense so I also moved it for them. Should be okay now

TBH there's a bit of cleanup to do about this... Many config files and build-related stuff are sooo outdated... Maybe gonna work on this.

But your work is nice thank you.

@williamdes
Copy link
Copy Markdown
Contributor Author

williamdes commented Oct 15, 2022

I figured out that the Makefile without the docker files would not make sense so I also moved it for them. Should be okay now

TBH there's a bit of cleanup to do about this... Many config files and build-related stuff are sooo outdated... Maybe gonna work on this.

But your work is nice thank you.

Thank you, feel free to ping me for review on build stuff

@Nek- Nek- merged commit 1961e11 into KnpLabs:master Nov 3, 2022
@Nek-
Copy link
Copy Markdown
Contributor

Nek- commented Nov 3, 2022

Thank you very much @williamdes !

@williamdes williamdes deleted the gitattrs branch November 3, 2022 18:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@Nek- Nek- Nek- approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Add a .gitattributes file

2 participants

@williamdes @Nek-