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

tools/scripts/phpunit-ls - Fix PhpStorm lookup of PHPUnit_Framework_TestCase #9535

Merged
merged 1 commit into from
Dec 19, 2016

Conversation

totten
Copy link
Member

@totten totten commented Dec 14, 2016

When using PhpStorm, code completion doesn't work smoothly on classes which
extend PHPUnit_Framework_TestCase because there are two copies: the real
copy and then a placeholder class used by tools/scripts/phpunit-ls.

Wrapping the placeholder copy in eval() prevents PhpStorm from
identifying.

…estCase

When using PhpStorm, code completion doesn't work smoothly on classes which
extend `PHPUnit_Framework_TestCase` because there are two copies: the real
copy and then a placeholder class used by `tools/scripts/phpunit-ls`.

Wrapping the placeholder copy in `eval()` prevents PhpStorm from
identifying.
@eileenmcnaughton
Copy link
Contributor

@agh1 I just tried to push Coleman to log a JIRA for a trivial change since you are doing release notes now & I feel we need to be tighter on issues to respect that. Is there a bar below which we don't need to - this seems trivial & obscure but ... I dunno

@agh1
Copy link
Contributor

agh1 commented Dec 15, 2016

@eileenmcnaughton I think the threshold for issues can be "notability". If it's something I might ever mention to a coworker, it deserves an issue, i.e.

  • "So this bug when you try to do X is no longer a problem,"
  • "Now you can modify Y from the front end," or
  • "The code in Z is refactored, so it should still work the same, but it might introduce a problem."

On the other hand, I'd never tell Tyrell, "Check out this exciting stuff @totten is wrapping with eval()." I think this can safely not be a Jira ticket.

@eileenmcnaughton
Copy link
Contributor

OK I'm merging this - the change only affects an obscure developer aspect and is very low risk so I feel Tim's testing / analysis is enough here

@eileenmcnaughton eileenmcnaughton merged commit bfee51f into civicrm:master Dec 19, 2016
@totten totten deleted the master-phpunit-phpstorm branch December 20, 2016 05:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants