An enhanced PR of PR#3 #6
Conversation
- Fixed default body column value. - Adopt to php8.
|
Hey @AbdullahFaqeir, thanks a lot for your PR! I'll take a look at it now. I'm not sure what you mean by adding hacktoberfest to the repo's topic? |
There was a problem hiding this comment.
I think it's awesome, thanks a lot for your work. Before I merge, I have 2 comments, could you please answer them? @AbdullahFaqeir
| } | ||
|
|
||
| return false; | ||
| return match ($chartType) { |
There was a problem hiding this comment.
hahahah glad I helped with this ^^'
| @@ -199,7 +201,7 @@ protected function ignoreCommentsAndPhpTags(array $token) | |||
|
|
|||
| protected function cleanOutput(string $output): string | |||
| { | |||
| $output = preg_replace('/(?s)(<aside.*?<\/aside>)|Exit: Ctrl\+D/ms', '$2', $output); | |||
| $output = preg_replace('/(?s)(<aside.*?<\/aside>)|Exit: {2}Ctrl\+D/ms', '$2', $output); | |||
There was a problem hiding this comment.
What's up this change? Not sure to understand?
There was a problem hiding this comment.
As the regex is parsing counts of 2, this assures the regex will work more accurately.
| $table->jsonb('body')->default('{"widgets":[]}'); | ||
| if (Config::get('database.defaults') === 'pgsql') { | ||
| $table->jsonb('body')->default('{"widgets":[]}'); | ||
| } else { |
There was a problem hiding this comment.
Have you had a chance to try this change with an actual mysql DB ?
There was a problem hiding this comment.
I did, I wrote a test cases locally and ran it and it passed to run the migration.
|
@JhumanJ you're most welcome. I don't know if you know about the hecktoberfest happening around the world for open source contributions, aside of me actually using this package, tho I'd like to count this PR in this festival, and in order to do that, you need the add 'hacktoberfest' keyword to the repo's topics. |
|
Awesome @AbdullahFaqeir ! Code looks good. I'll give it a try on some of my applications and merge the PR by tomorrow. I believe I added the hacktoberfest keywords on the package, but it it's not what you expected, please let me know. |
|
Great @JhumanJ I'll be waiting for the merge and the new release so I can use it. And yes the keywords are perfect, once you approve and merge the PR it'll be count for me in the fest scoreboard ^^'. |
|
@JhumanJ did you manage to review and merge this PR? |
Just did now @AbdullahFaqeir , thanks for the heads up and for your contribution 🙏 |
You're most welcome bro 😎 |
@JhumanJ if you can please add hacktoberfest to this repo's topics.