[GitHub] cordova-lib pull request: CB-10430 Allow to modify events source

2016-02-09 Thread vladimir-kotikov 
Github user vladimir-kotikov closed the pull request at:

https://github.com/apache/cordova-lib/pull/370


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-lib pull request: CB-10430 Allow to modify events source

2016-02-09 Thread vladimir-kotikov 
Github user vladimir-kotikov commented on the pull request:

https://github.com/apache/cordova-lib/pull/370#issuecomment-181906497
  
Closing this for now. I'll reopen another PR with updated implementation.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-lib pull request: CB-10430 Allow to modify events source

2016-02-04 Thread nikhilkh 
Github user nikhilkh commented on the pull request:

https://github.com/apache/cordova-lib/pull/370#issuecomment-180179865
  
Yes, we could fix all callers of it - but ideally, 'log' messages from 
`cordova-common` can be surfaced and reported. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-lib pull request: CB-10430 Allow to modify events source

2016-02-04 Thread vladimir-kotikov 
Github user vladimir-kotikov commented on the pull request:

https://github.com/apache/cordova-lib/pull/370#issuecomment-180026635
  
Guys, just a couple of ideas, based on discussion with Nikhil:
1, Instead of overwriting `events` instance in common, platform might set a 
reference to `events` on `global` object, which `cordova-common` will check at 
runtime and use if available
2. Platform might create a proxy - just subscribe to and redirect 
`cordova-common`'s `events` to own instance - this one have an advantage - we 
would not need for any special logic in `common` for that


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-lib pull request: CB-10430 Allow to modify events source

2016-02-04 Thread purplecabbage 
Github user purplecabbage commented on the pull request:

https://github.com/apache/cordova-lib/pull/370#issuecomment-180030165
  
I am still not seeing the value in this.
When is this an issue? Who does it affect? 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-lib pull request: CB-10430 Allow to modify events source

2016-02-04 Thread nikhilkh 
Github user nikhilkh commented on the pull request:

https://github.com/apache/cordova-lib/pull/370#issuecomment-180028239
  
I like the idea of a global.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-lib pull request: CB-10430 Allow to modify events source

2016-02-04 Thread nikhilkh 
Github user nikhilkh commented on the pull request:

https://github.com/apache/cordova-lib/pull/370#issuecomment-180036146
  
https://issues.apache.org/jira/browse/CB-10430 has details of one scenario 
where verbose logs from the cordova-common do not show up. In particular, the 
processes that we spawn using superspawn as part of a build can fail and we get 
an error without context of the command that was run. Ideally, --verbose build 
logs will have details of the external command that was executed with the 
correct arguments.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-lib pull request: CB-10430 Allow to modify events source

2016-02-04 Thread purplecabbage 
Github user purplecabbage commented on the pull request:

https://github.com/apache/cordova-lib/pull/370#issuecomment-180043070
  
In this case, why not emit the event that you are calling spawn with 
arguments a,b,c 

```
function execSpawn(command, args, resultMsg, errorMsg) {
events.emit( ... )
return superspawn.spawn(command, args).then(function(result) {
   ...
```


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-lib pull request: CB-10430 Allow to modify events source

2016-02-03 Thread vladimir-kotikov 
Github user vladimir-kotikov commented on the pull request:

https://github.com/apache/cordova-lib/pull/370#issuecomment-179297923
  
No, this is not a part of API. However, this might be critical for some 
external tools to get this information.

@nikhilkh, what do you think about this?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-lib pull request: CB-10430 Allow to modify events source

2016-02-03 Thread nikhilkh 
Github user nikhilkh commented on the pull request:

https://github.com/apache/cordova-lib/pull/370#issuecomment-179384632
  
I understand that this approach is not an ideal pattern - Anyone has better 
ideas?

We do need this to work to ensure that --verbose logs that we produce have 
details of activity in cordova-common. In particular, logging of all the 
commands that we spawn (with arguments). It really helps in diagnosing failures 
and bugs. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-lib pull request: CB-10430 Allow to modify events source

2016-02-02 Thread dblotsky 
Github user dblotsky commented on the pull request:

https://github.com/apache/cordova-lib/pull/370#issuecomment-178861517
  
Is this a part of our external API? If not, then why should there be a way 
to get them without the debugger if it's an internal feature?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-lib pull request: CB-10430 Allow to modify events source

2016-02-02 Thread dblotsky 
Github user dblotsky commented on the pull request:

https://github.com/apache/cordova-lib/pull/370#issuecomment-178555686
  
@vladimir-kotikov I guess my point is: why is it necessary for internal 
`cordova-common` events to reach platform code?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-lib pull request: CB-10430 Allow to modify events source

2016-02-02 Thread vladimir-kotikov 
Github user vladimir-kotikov commented on the pull request:

https://github.com/apache/cordova-lib/pull/370#issuecomment-178554295
  
@dblotsky, @purplecabbage, thanks for feedback. Agree, this probably smells 
fishy. However, the only one option i see apart from this one is to setup a 
proxy to transmit events from `cordova-common` to another `EventEmitter` 
instance (which was passed to `PlatformApi` constructor), when necessary.

All other solutions, that came into my mind, break `events` API and hence 
require some global changes in cordova (as the `events` is used really widely).

However, this PR doesn't change a public API for this and seems to fix the 
issue with missing events from `cordova-common` internals without affecting any 
other cases, so it might make sense to get this in as an intermediate fix and 
then start looking into redesigning cordova `events` mechanics.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-lib pull request: CB-10430 Allow to modify events source

2016-02-02 Thread vladimir-kotikov 
Github user vladimir-kotikov commented on the pull request:

https://github.com/apache/cordova-lib/pull/370#issuecomment-178561020
  
 Because we've been logging these events all the time before cordova 
refactoring. Many of these events doesn't have much value though, and could be 
safely ignored, but, for example, the command and arguments, used by superspawn 
are pretty important information IMO, and there should be a way to get it 
without using debugger


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-lib pull request: CB-10430 Allow to modify events source

2016-02-01 Thread dblotsky 
Github user dblotsky commented on the pull request:

https://github.com/apache/cordova-lib/pull/370#issuecomment-178305625
  
Overriding an implementation's logging with your own sounds like a 
roundabout way to get what you need. This sounds like it's exposing internal 
events for the purposes of external debugging, which is best solved by using a 
debugger, not with a custom logging mechanism.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-lib pull request: CB-10430 Allow to modify events source

2016-02-01 Thread vladimir-kotikov 
GitHub user vladimir-kotikov opened a pull request:

https://github.com/apache/cordova-lib/pull/370

CB-10430 Allow to modify events source

This fixes [CB-10430](https://issues.apache.org/jira/browse/CB-10430).
This change is needed to allow other tools to replace default 
`EventEmitter` instance to custom one (e.g. platform might want to replace 
default `EventEmitter` with the one provided by caller in order to pass these 
events to upstream code)

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/MSOpenTech/cordova-lib CB-10430

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/cordova-lib/pull/370.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #370


commit 66ee6a90ad30b708c1ebcf2abf01773d8ab42c6e
Author: Vladimir Kotikov 
Date:   2016-01-12T07:56:59Z

CB-10430 Allow to modify events source




---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-lib pull request: CB-10430 Allow to modify events source

2016-02-01 Thread purplecabbage 
Github user purplecabbage commented on the pull request:

https://github.com/apache/cordova-lib/pull/370#issuecomment-178414449
  
Yeah, I too am not entirely behind this.
It seems like an anti-pattern here, and a consequence of 
require('../events') only being executed once per module.  This essentially 
makes it a global object that may change at runtime.



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org