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

[travis] Get tested PHP version from Travis CI #1255

Closed
wants to merge 9 commits into from
Closed

[travis] Get tested PHP version from Travis CI #1255

wants to merge 9 commits into from

Conversation

peter-gribanov
Copy link
Contributor

@peter-gribanov peter-gribanov commented Nov 8, 2017

For issue #819

In this PR i add badge for get PHP version only from Travis CI.
Also need to add a badges for get version from Composer and PHP-Eye.
I plan to make it separate PR.

I added tests, but they are dangerous.
The success of the tests depends on Symfony.
If you have any ideas how to fix this problem, tell me please.

PHP

@paulmelnikow
Copy link
Member

Hi, thanks for this. I appreciate what you're trying to do here.

The implementation is pretty involved. Would reading values from .travis.yml be sufficient?

It seems like a tests | success, from Travis API, in conjunction with PHP | 7.1, from .travis.yml, would provide sufficient signal to the user, without needing to fetch the results of all the Travis tests. It would be simpler to understand and run a lot faster.

@paulmelnikow paulmelnikow added the service-badge Accepted and actionable changes, features, and bugs label Nov 8, 2017
@peter-gribanov
Copy link
Contributor Author

Successfully passed tests are the guarantor of that that the application works on this PHP version.
On the same principle, the PHP-Eye works.

In order not to pull out data from the Travis CI, you can use Composer or PHP-Eye

@peter-gribanov
Copy link
Contributor Author

I can make a separate badge which will read data only from .travis.yml.

@paulmelnikow
Copy link
Member

Successfully passed tests are the guarantor of that that the application works on this PHP version.

If the tests are configured for three versions, and the tests pass, logically it implies that the tests pass on those three versions. The difference between that and a post facto guarantee seems pedantic. It's just a badge, not a deploy trigger.

On the same principle, the PHP-Eye works.

Is the PHP-Eye Travis data different from what you're extracting in #1258?

@peter-gribanov
Copy link
Contributor Author

Is the PHP-Eye Travis data different from what you're extracting in #1258?

As far as i can tell, Yes. They check the result of starting the build.

@peter-gribanov
Copy link
Contributor Author

peter-gribanov commented Nov 10, 2017

After merge #1258, i can simplify this implementation and, for example, rename it to Tested.

Tested

In my opinion this is simpler and more obvious than:

Yii 1.1.19

@peter-gribanov peter-gribanov changed the title Get PHP version from Travis CI [travis] Get PHP version from Travis CI Dec 6, 2017
@peter-gribanov peter-gribanov changed the title [travis] Get PHP version from Travis CI [travis] Get tested PHP version from Travis CI Dec 6, 2017
@paulmelnikow
Copy link
Member

Okay, so #1258 is now merged. I appreciate the work you put into that, and into this. Though the endpoint from #1258 for "Tested versions based on Travis" is not perfect, as you've observed, it's good enough, given the "perfect" implementation would need to make several requests. If you can pull the data you want more simply from PHP-Eye I'd certainly be open to that. Also, to adding Composer if you'd like. Thanks again!

@peter-gribanov
Copy link
Contributor Author

@paulmelnikow I'll definitely make a badge for PHP-Eye and I'll redo this badge when i have time.

@paulmelnikow
Copy link
Member

Thanks for understanding!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
service-badge Accepted and actionable changes, features, and bugs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants