-
Notifications
You must be signed in to change notification settings - Fork 0
Fix Synapse comments (#temp-table is not a comment) #3
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
Conversation
27bd1ca
to
ad2aec7
Compare
ad2aec7
to
d5dad3c
Compare
@@ -1,5 +1,10 @@ | |||
language: php | |||
php: | |||
- 5.6 | |||
- 7.1 | |||
- 7.4 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pridal som testovanie aj oproti novsim PHP verziam.
@@ -122,6 +122,13 @@ class SqlFormatter | |||
// If not defined, it will be determined automatically | |||
public static $cli; | |||
|
|||
// Comments | |||
public static $comment_tokens = [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Vyrazy pre komentare je mozne uz definovat, ... ak je iba jedna polozka, tak sa berie, ze je to riadkovy komentar, ... inak cely blok.
|
||
if ($last === false) { | ||
$last = strlen($string); | ||
$found = false; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Implementaciu komentarov som zovseobecnil, ... je to vsetko pokryte testami
@@ -113,6 +112,25 @@ function testCacheStats() { | |||
$this->assertGreaterThan(1,$stats['hits']); | |||
} | |||
|
|||
public function testSynapseTempTables() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Teda uz sa to bude dalo pouzit v synapse-transformations
... a vypnut #
ako znak pre comment -> lebo to je v Synapse temp tabulka.
@ondrajodas mozes sa prosim na to pozriet? Je to blocker pre CSAS. |
@webrouse do hoďky na to kouknu |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cajk 👍
} | ||
|
||
} | ||
if ($found) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tohle by šlo dát přímo do toho foreache a nemusíš to kontrolovat ještě zvlášť
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
https://testing.googleblog.com/2017/06/code-health-reduce-nesting-reduce.html
Myslim si, ze je spravnejsie snazit sa mat co najmenej urovni kodu. Je to tak lachsie citatelne a menej nachylne na chyby.
V prvej casti sa zisti ci je to coment. V druhej casti sa to spracuje.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Myslim si, ze je spravnejsie snazit sa mat co najmenej urovni kodu
- souhlasím, ale když už tam ten if je :)
Je to tak lachsie citatelne a menej nachylne na chyby.
- v pohodě já nejsem proti :)
lib/SqlFormatter.php
Outdated
/** @var string $end */ | ||
$endPos = strpos($string, $end, strlen($start)); | ||
$last = $endPos ? | ||
($endPos + ($end === "\n" ? 0 : strlen($end))) : strlen($string); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🤯
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hej, mas pravdu :)
Spravil som to viac citatelnejsie:
94fee9b
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
jj tak to je lepší 👍 díky
Jira: https://keboola.atlassian.net/browse/COM-570
Changes: