Skip to content

Comments

Adds include support to match tests before calling run().#8

Open
elsigh wants to merge 1 commit intotagview:masterfrom
elsigh:master
Open

Adds include support to match tests before calling run().#8
elsigh wants to merge 1 commit intotagview:masterfrom
elsigh:master

Conversation

@elsigh
Copy link

@elsigh elsigh commented Apr 27, 2015

Updated PR because $$ means I need to name the variable $include not $include_regex. Tested that it's working.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ouch, these variable-variables name looks so creepy. I know it is a bit more verbose, but wouldn't you mind to use change it to something like:

switch($configuration) {
  case 'include': 
    $include_files = $value;
    break;
  case 'include_report':
    $include_report = $value
    break
}

Copy link
Author

Choose a reason for hiding this comment

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

Done. Yeah, I felt particularly PHP-y using the $$ ;0 This is nicer.

@reu
Copy link
Contributor

reu commented Apr 30, 2015

Looks great IMO! I just need to give a try before merging.

@elsigh
Copy link
Author

elsigh commented May 5, 2015

As a side note, it would be cool if there were unit tests for the project, just because manual testing is kind of a pain.

@elsigh
Copy link
Author

elsigh commented May 5, 2015

Fixes #5

@elsigh
Copy link
Author

elsigh commented May 11, 2015

cool, thanks! I put up the inclusion of the GoTestEngine as well.

On Wed, Apr 29, 2015 at 6:45 PM, Rodrigo Navarro notifications@github.com
wrote:

Looks great IMO, I just need to give a try before merging.


Reply to this email directly or view it on GitHub
#8 (comment)
.

@elsigh
Copy link
Author

elsigh commented May 29, 2015

any chance you want to merge this still? I can keep using my own repo, but I like using a single one ;)

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.

2 participants