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

Created Parser Plugin for validation errors issue#675 #716

Merged
merged 5 commits into from
Sep 15, 2017

Conversation

jaynarayan89
Copy link
Contributor

No description provided.

{
return $validator->listErrors();
}
else
Copy link
Member

Choose a reason for hiding this comment

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

no need else as already return early

Copy link
Member

Choose a reason for hiding this comment

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

Agreed. Please remove the else here.

Copy link
Member

@lonnieezell lonnieezell left a comment

Choose a reason for hiding this comment

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

Take care of that one small change and add a test, and some docs in /user_guide_source/general/view_parser.rst and this should be good.

{
return $validator->listErrors();
}
else
Copy link
Member

Choose a reason for hiding this comment

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

Agreed. Please remove the else here.

@jaynarayan89
Copy link
Contributor Author

Ok. I will make changes.
Another thing I want to point out that we should have plugin for viewcell and form helper methods as well.
If you agree I would happy to help.

@jim-parry
Copy link
Contributor

@jaynarayan89 Just pointing out that we would like to see contributions GPG-signed ... https://bcit-ci.github.io/CodeIgniter4/contributing/signing.html

@jaynarayan89
Copy link
Contributor Author

Hi , I accidentally merged another branch with this and messed up commits. I tried to revert but not succeed. what should I do now?
I am learning github. sorry for the trouble.

@jim-parry ok. I will do it.

@jim-parry
Copy link
Contributor

@jaynarayan89 Your last "good" (pre patch2) merge shows a commit hash tag starting with "17fa3d4". Assuming that is the "good" point you want, you should be able to revert to that with "git reset --hard 17fa3d4...", and then "git push -f origin patch-2" to fix your feature branch.

@jaynarayan89
Copy link
Contributor Author

Thanks a lot @jim-parry
I need another help regarding Unit testing. when i run phpunit in my root folder it shows:

PHPUnit 3.7.21 by Sebastian Bergmann.
Configuration read from D:\xampp\htdocs\ci4.dev\CodeIgniter4\phpunit.xml
Time: 921 ms, Memory: 10.00MB

←[30;43m←[2KNo tests executed!
←[0m←[2K
Generating code coverage report in Clover XML format ... done

@jim-parry
Copy link
Contributor

jim-parry commented Sep 11, 2017

@jaynarayan89 I run unit tests with the following command from my project root...
phpunit --exclude-group DatabaseLive --colors --coverage-text=tests/coverage.txt --coverage-html=tests/coverage/

ps - I am using phpunit 6.1.3 ... it looks like yours is a bit out-of-date :-/

@jaynarayan89
Copy link
Contributor Author

@jim-parry
Again same messages.
D:\xampp\htdocs\ci4.dev\CodeIgniter4>phpunit --exclude-group DatabaseLive --colo
rs --coverage-text=tests/coverage.txt --coverage-html=tests/coverage/
PHPUnit 3.7.21 by Sebastian Bergmann.

Configuration read from D:\xampp\htdocs\ci4.dev\CodeIgniter4\phpunit.xml
Time: 505 ms, Memory: 10.00MB

←[30;43m←[2KNo tests executed!
←[0m←[2K
Generating code coverage report in Clover XML format ... done
Generating code coverage report in HTML format ... done

@jaynarayan89
Copy link
Contributor Author

@jim-parry
Regarding phpunit version:
I installed phpunit via composer an hour ago. using this command
composer require --dev phpunit/phpunit ^6.2
however it installed old version. I tried but cant install latest version.

@jim-parry
Copy link
Contributor

@jaynarayan89 Rather than clutter up this commit history, can you start a thread with me on our slack channel? thanks ... http://codeignitersignup.ciblox.com/

@jaynarayan89
Copy link
Contributor Author

@lonnieezell
Please review changes

@lonnieezell
Copy link
Member

I will, but you have to give me more than an hour. :) It's the middle of my work day here, and, unfortunately, I don't make a living developing this so won't be until tonight (hopefully!) before I can check it. Though the way the last couple of weeks have been going, I'll probably be working really late.

@lonnieezell lonnieezell merged commit a161ebd into codeigniter4:develop Sep 15, 2017
@jaynarayan89 jaynarayan89 deleted the patch-2 branch September 15, 2017 17:03
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.

4 participants