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

Exclude directories from the Composer archive #58

Merged
merged 1 commit into from
May 19, 2020

Conversation

mnapoli
Copy link
Contributor

@mnapoli mnapoli commented May 19, 2020

Proposed Changes

This results in a lighter package when it is installed in vendor in consuming projects. This makes a difference on AWS Lambda where the package size is limited, and a smaller package implies better performances for cold starts.

See https://madewithlove.com/gitattributes/ for more information.

This results in a lighter package when it is installed in `vendor` in consuming projects. This makes a difference on AWS Lambda where the package size is limited, and a smaller package implies better performances for cold starts.

See https://madewithlove.com/gitattributes/ for more information.
@mnapoli mnapoli requested a review from hollodotme as a code owner May 19, 2020 07:52
@codecov
Copy link

codecov bot commented May 19, 2020

Codecov Report

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

Impacted file tree graph

@@            Coverage Diff            @@
##             master      #58   +/-   ##
=========================================
  Coverage     97.96%   97.96%           
  Complexity      253      253           
=========================================
  Files            18       18           
  Lines           639      639           
=========================================
  Hits            626      626           
  Misses           13       13           

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 f67b10b...aeff9ac. Read the comment docs.

Copy link
Owner

@hollodotme hollodotme left a comment

Choose a reason for hiding this comment

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

Thanks @mnapoli, this is handy. I didn't know that it also affects the composer releases.

Only one thing though: Can you remove the bin/ directory from .gitattributes, because bin/fcgiget gets linked on composer install, so this would be a BC break, see: https://github.com/hollodotme/fast-cgi-client/blob/v3.1.1/composer.json#L20

@hollodotme hollodotme added this to the v3.1.2 milestone May 19, 2020
@hollodotme hollodotme changed the base branch from master to development May 19, 2020 13:27
@hollodotme
Copy link
Owner

As discussed with @mnapoli via DM:

  • I'll merge this PR which will exclude the bin/ directory from releases installed by composer
  • I'll remove the link of bin/fcgiget in composer.json and document it as a security fix (because bin/fcgiget accepts any accessible fastCGI endpoint)
  • I'll release it as v3.1.2

@hollodotme hollodotme self-requested a review May 19, 2020 13:31
Copy link
Owner

@hollodotme hollodotme left a comment

Choose a reason for hiding this comment

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

Approved, see conversation

@hollodotme hollodotme merged commit 689851b into hollodotme:development May 19, 2020
@hollodotme hollodotme mentioned this pull request May 19, 2020
@mnapoli mnapoli deleted the patch-1 branch May 19, 2020 15:38
@mnapoli
Copy link
Contributor Author

mnapoli commented May 19, 2020

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants