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 .gitattributes #688

Merged
merged 1 commit into from
Aug 12, 2017
Merged

Add .gitattributes #688

merged 1 commit into from
Aug 12, 2017

Conversation

garak
Copy link
Contributor

@garak garak commented Aug 12, 2017

This is also fixing #639

This is also fixing doctrine#639
@mikeSimonson
Copy link
Contributor

@garak Thanks

@mikeSimonson mikeSimonson merged commit 59e35a2 into doctrine:master Aug 12, 2017
@garak garak deleted the patch-1 branch August 12, 2017 10:18
@alcaeus
Copy link
Member

alcaeus commented Aug 13, 2017

I'm not sure this is a good idea. This will cause a severe difference between running composer install --prefer-source and composer install --prefer-dist. IMO it would be better to split up into a lib and tests folder as we do on many other repositories (and as #641 suggested). What do you think @mikeSimonson?

@garak
Copy link
Contributor Author

garak commented Aug 13, 2017

What would be the problem with such difference?

@alcaeus
Copy link
Member

alcaeus commented Aug 13, 2017

For starters, the issue you linked above is only fixed for some people. People with source installs still have the test sources available in their autoloader.

@garak
Copy link
Contributor Author

garak commented Aug 13, 2017

Ah OK, I didn't realize that. Anyway, the issue is still open

@Ocramius
Copy link
Member

@alcaeus we do it on all other repos and it's fine. The tests are NOT public API anyway, so whoever is installing the package as a dependency shouldn't care about whether they are there or not.

@mikeSimonson
Copy link
Contributor

Well the next steps are

  • to reorganize the repo with the tests outside the rest of the src folder
  • and the autoload-dev feature of composer.json

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.

4 participants