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.

Sign up for GitHub

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

Jump to bottom

drop versions below PHP 7.2 #19

Merged
merged 10 commits into from
Jul 24, 2019
Merged

drop versions below PHP 7.2 #19

merged 10 commits into from
Jul 24, 2019
+17 −22

Conversation

abbadon1334
Copy link
Collaborator

i think this must be done

@abbadon1334
drop versions below PHP 7.2

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
GPG key ID: 4AEE18F83AFDEB23
Expired
Verified
Learn about vigilant mode
ca4b93e 
i think this must be done
@abbadon1334 abbadon1334 mentioned this pull request Jul 12, 2019
Closed
@DarkSide666
Copy link
Member

Why not support PHP 7.0 and 7.1 ?

@DarkSide666 DarkSide666 self-requested a review July 15, 2019 08:09
@abbadon1334
Copy link
Collaborator Author

Why not support PHP 7.0 and 7.1 ?

because of this : https://www.php.net/supported-versions.php, prior of this i see that atk4/data drop support for php below 7.2.

We can leave this to support >7.0 and let composer resolve requirements if used with other packages.

I open this issue while I was showing to one of my dev the library and i see that it was required 5.6.
After that i see yours on same subject but with drop for 7.0 and i decide to leave it open.

@DarkSide666
Copy link
Member

DarkSide666 commented Jul 15, 2019 •
edited
Loading

Yeah, probably you're right about 7.2. If data don't support php <7.2, then it's useless to try to run api on 7.0 or 7.1.

Please also add fix for travis.yml (remove 7.0, 7.1 and maybe add 7.3) and let @romaninsh decide what to do with this.

@DarkSide666 DarkSide666 requested a review from romaninsh July 15, 2019 09:48
@abbadon1334
Drop Travis tests for PHP version lower than 7.2

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
GPG key ID: 4AEE18F83AFDEB23
Expired
Verified
Learn about vigilant mode
4dad354
@abbadon1334
fix travis : after & before _script

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
GPG key ID: 4AEE18F83AFDEB23
Expired
Verified
Learn about vigilant mode
7fdc3ce
@abbadon1334
Copy link
Collaborator Author

@romaninsh in travis i removed the lines targeting php version for coverage, please check it.

@abbadon1334
Add Travis test PHP version 7.3

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
GPG key ID: 4AEE18F83AFDEB23
Expired
Verified
Learn about vigilant mode
5218f50
@DarkSide666
Copy link
Member

@abbadon1334 Why did you remove code coverage sending? I think it was fine, just needed to change 7.0 to 7.2.

@abbadon1334
ops... set back coverage

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
GPG key ID: 4AEE18F83AFDEB23
Expired
Verified
Learn about vigilant mode
3bea5be
@abbadon1334
travis EOF

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
GPG key ID: 4AEE18F83AFDEB23
Expired
Verified
Learn about vigilant mode
4b32369 
/dev/fd/63: eval: line 1257: unexpected EOF while looking for matching `]'
/dev/fd/63: eval: line 1258: syntax error: unexpected end of file
@abbadon1334
Try to fix - Travis
47b0ce0
@abbadon1334
merge changes
f79e5a5
@abbadon1334
copy before and after script from atk4/ui
3b662bf
@abbadon1334
fix2 travis coverage
30d6b45
@codecov
Copy link

codecov bot commented Jul 23, 2019

Codecov Report

❗ No coverage uploaded for pull request base (develop@c1a595c). Click here to learn what that means.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##             develop      #19   +/-   ##
==========================================
  Coverage           ?   34.75%           
  Complexity         ?       76           
==========================================
  Files              ?        1           
  Lines              ?      164           
  Branches           ?        0           
==========================================
  Hits               ?       57           
  Misses             ?      107           
  Partials           ?        0

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 c1a595c...30d6b45. Read the comment docs.

2 similar comments
@codecov
Copy link

codecov bot commented Jul 23, 2019

Codecov Report

❗ No coverage uploaded for pull request base (develop@c1a595c). Click here to learn what that means.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##             develop      #19   +/-   ##
==========================================
  Coverage           ?   34.75%           
  Complexity         ?       76           
==========================================
  Files              ?        1           
  Lines              ?      164           
  Branches           ?        0           
==========================================
  Hits               ?       57           
  Misses             ?      107           
  Partials           ?        0

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 c1a595c...30d6b45. Read the comment docs.

@codecov
Copy link

codecov bot commented Jul 23, 2019

Codecov Report

❗ No coverage uploaded for pull request base (develop@c1a595c). Click here to learn what that means.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##             develop      #19   +/-   ##
==========================================
  Coverage           ?   34.75%           
  Complexity         ?       76           
==========================================
  Files              ?        1           
  Lines              ?      164           
  Branches           ?        0           
==========================================
  Hits               ?       57           
  Misses             ?      107           
  Partials           ?        0

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 c1a595c...30d6b45. Read the comment docs.

@abbadon1334
Copy link
Collaborator Author

Coverage is back, sorry. I see a losing of coverage, if you want to merge this PR, i will look further in the issue and i will open another one only to increase coverage, because i think api must have higher rate of testing.
Moreover with all the microservice request of the community and with last addiction of atk4/data action probably will be used more.

DarkSide666
DarkSide666 approved these changes Jul 24, 2019
View reviewed changes
Copy link
Member

@DarkSide666 DarkSide666 left a comment

Choose a reason for hiding this comment

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

now all looks good, merging

@DarkSide666 DarkSide666 merged commit 256f659 into atk4:develop Jul 24, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@DarkSide666 DarkSide666

@romaninsh romaninsh

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@abbadon1334 @DarkSide666