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

Fix for issue #220 #221

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open

Fix for issue #220 #221

wants to merge 8 commits into from

Conversation

umatz
Copy link

@umatz umatz commented Jun 29, 2017

Corrected wrong quotation marks.

@chris001
Copy link

@umatz Congrats on your first pull request!
If you edit also in your fork's same branch patch-1 this file:
https://github.com/umatz/Provisioner/blob/patch-1/endpoint/yealinkv70/t2x/line_keys_28.json
...then this pull request will succeed the Travis build.

@umatz
Copy link
Author

umatz commented Jun 29, 2017 via email

@chris001
Copy link

@umatz Very good!
Travis is still complaining about this error in phone.php line 57:

There was 1 error:

1) StackTest::testreplacementall

Undefined index: T27

/home/travis/build/provisioner/Provisioner/endpoint/yealinkv70/t2x/phone.php:57

@chris001
Copy link

@umatz
You should add an entry in the array $model_suffixes for T27
https://github.com/umatz/Provisioner/blob/patch-1/endpoint/yealinkv70/t2x/phone.php

@umatz
Copy link
Author

umatz commented Jun 29, 2017

I am not certain, whether I can use any unique id in this array ... but the checks are passing now.

@chris001
Copy link

@umatz Yes it's only failing for php 5.2 which doesn't exist any longer in Travis and so 5.2 should be removed from https://github.com/umatz/Provisioner/blob/patch-1/.travis.yml

@umatz umatz mentioned this pull request Jun 29, 2017
@chris001
Copy link

Great, it's passing now. Also add php 5.5 5.6 7.0 and 7.1 because these recent php versions which the majority of well maintained systems are running right now https://github.com/umatz/Provisioner/blob/patch-1/.travis.yml

@chris001
Copy link

@umatz
The php 7.0 and 7.1 environment has a different class name for phpunit tests - supposedly.
The workaround to test php 7.x is to use phpunit 4.x or 5.x instead of the newest phpunit 6.x which Travis loads by default...

@umatz
Copy link
Author

umatz commented Jul 1, 2017

@chris001
Since FreePBX doesn't run under php 7.x I guess its ok to leave it at this (tests passing with php 5.5 and 5.6).

@chris001
Copy link

chris001 commented Jul 1, 2017

OK. Sangoma FreePBX team really should upgrade the PHP code soon to be compatible with PHP 7.x because PHP 5.6 has ended active support 5 months ago and is security support only for 17 more months, at that time 5.6 will be dead and subject to security attacks on irreparable vulnerabilities.

@ajoah
Copy link

ajoah commented Jul 3, 2017

Are you sure is it the good T27 suffix ? Strange that T26 suffix is 04 and T27 suffix is 27

@umatz
Copy link
Author

umatz commented Jul 3, 2017

Actually not, see my comment above. I could not figure out, what the ID in this array refers to.

@tm1000
Copy link
Contributor

tm1000 commented Jul 3, 2017

@chris001
Copy link

chris001 commented Jul 6, 2017

@tm1000
The T27 model and its suffix aren't listed on that forum page.
It's listed here:
http://support.yealink.com/documentFront/forwardToDocumentDetailPage?documentId=36
It seems to be 45!
It's always the first number in the firmware filename.

@umatz
Copy link
Author

umatz commented Jul 6, 2017

@chris001
Copy link

chris001 commented Jul 6, 2017

@umatz Are they same enough that you can just add entries for both T27P and T27G.
Also, your link is missing the f on the end .pdf

@umatz
Copy link
Author

umatz commented Jul 8, 2017

The only difference between the 27P and 27G seems to be that the G supports Gigabit Ethernet vs 100 MBit on the 27P.

@Vodochnik
Copy link

Just wondering WHY is this excellent work still not merged...

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.

5 participants