thervh70 / ContextProject_RDD

1 stars 0 forks source link

140 tabchange evnt #162

Closed MathiasMeuleman closed 8 years ago

MathiasMeuleman commented 8 years ago

Will close #140

Added a TabchangeEvent. Is fired whenever a tab is changed (for obvious reasons)

coveralls commented 8 years ago

Coverage Status

Coverage decreased (-0.04%) to 93.017% when pulling 70b8e6b89cab563f5118fa6c7951757dc50f7c23 on 140-tabchange-evnt into 9ff62c72ccfbd7f683c53120f1d973f0193d2b14 on dev.

Exclaminator commented 8 years ago

LGTM

mpsijm commented 8 years ago

When I use Ctrl+Tab and flip through my entire tab list (you know how many I have :innocent: ) it doesn't log much? :P

MathiasMeuleman commented 8 years ago

Of course it doesn't, the plugin doesn't log anything when the user is on other tabs. The only thing that can and should be logged is a tab change from or to the PR.

mpsijm commented 8 years ago

I'm not sure whether that was the initial idea in that case :P I thought we needed to log all tab changes. Maybe check the meeting minutes?

MathiasMeuleman commented 8 years ago

Meeting minutes are inconslusive. But, since only a timestamp is logged and not the previous tab/next tab, it is absolutely not useful to log every tab change, since this doesn't provide any information.

mpsijm commented 8 years ago

It indeed doesn't matter whether next/previous tab is logged, I only meant to say that I think that all tab changes should be logged, and Ctrl+Tabbing through my tabs was an easy way to figure out that that currently does not happen :

thervh70 commented 8 years ago

Please fix the comment issue stated in the code. Then it is ready to merge!

mdingjan commented 8 years ago

I agree with the comments that already have been placed. Make sure to fix these, then this PR can be merged.

mdingjan commented 8 years ago

Thanks for processing the feedback; will merge!

coveralls commented 8 years ago

Coverage Status

Coverage increased (+0.2%) to 93.226% when pulling f0a82a98c666f54801ec49476aaa054ea7b19a08 on 140-tabchange-evnt into 9ff62c72ccfbd7f683c53120f1d973f0193d2b14 on dev.