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

improve composer.json and travis tests #57

Merged
merged 8 commits into from
Feb 7, 2020
Merged

improve composer.json and travis tests #57

merged 8 commits into from
Feb 7, 2020

Conversation

Tobion
Copy link
Contributor

@Tobion Tobion commented Feb 5, 2020

- make dependencies explicit
- remove compatibility with unmaintained symfony versions
- define branch alias
- remove bin config as the binaries from this package are only proxies and do not need to be exposed in other apps
@Tobion Tobion changed the title improve composer.json improve composer.json and travis tests Feb 5, 2020
Copy link
Member

@Oliboy50 Oliboy50 left a comment

Choose a reason for hiding this comment

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

👋 thanks for the PR

LGTM

TODO: fix test for M6WebAmqpExtension with symfony 4+ as the service are private the tested service gets removed

indeed 👍
fact is that we're trying to move from atoum to phpunit (since Symfony is "recommending" the use of phpunit in its ecosystem), so maybe we could also use phpunit in this project too (don't know if it could help)

anyway, the Travis build shows that php7.0 and php7.1 are green, so I think this PR doesn't break anything

@Tobion
Copy link
Contributor Author

Tobion commented Feb 6, 2020

@Oliboy50 this is ready. tests pass. see also my last point in the original description. you could merge this and release it as a new patch release. then the next step would be to support SF 5.

Copy link
Member

@Oliboy50 Oliboy50 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 again 👏

i'm still not convinced about the .gitattributes file, but I think it won't hurt anyway 👌

I'll let (at least) one of my colleagues approve, merge and release your PR
(the M6Web review policy requires at least 2 approvals before merging)

Copy link
Contributor

@b-viguier b-viguier left a comment

Choose a reason for hiding this comment

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

Really nice 👏 👍
Thanks a lot

/Dockerfile export-ignore
/docker export-ignore
/docker-compose.yml export-ignore
/travis export-ignore
Copy link
Contributor

Choose a reason for hiding this comment

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

By curiosity, in which case is it interesting to prevent the export of these files? 🤔
Is it to provide lighter composer package? If so, what about src/AmqpBundle/Tests/?
Just wondering 😄

Copy link
Member

Choose a reason for hiding this comment

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

Is it to provide lighter composer package?

indeed :D

#52 (comment)

@lnahiro lnahiro merged commit d3c1d4e into BedrockStreaming:master Feb 7, 2020
@lnahiro
Copy link
Contributor

lnahiro commented Feb 7, 2020

release v3.1.2

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.

6 participants