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

TIMOB-18066 Support remote debugging / inspecting of Android webviews #6365

Closed
wants to merge 1 commit into from

Conversation

timanrebel
Copy link
Contributor

Enable remote debugging of Android WebViews when App is debuggable

https://jira.appcelerator.org/browse/TIMOB-18066?filter=-2

Enable remote deubgging of Android WebViews when App is debuggable
@timanrebel timanrebel changed the title #TC-5016 Support remote debugging / inspecting of Android webviews TIMOB-18066 Support remote debugging / inspecting of Android webviews Dec 4, 2014
@@ -180,6 +181,14 @@ private boolean isHTCSenseDevice()
public TiUIWebView(TiViewProxy proxy)
{
super(proxy);

// We can only support debugging in API 19 and higher
if(Build.VERSION.SDK_INT >= Build.VERSION_CODES.KITKAT) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Indentation needs to be fixed and please leave a space between "if" and "(".

@hieupham007
Copy link
Contributor

Code reviewed. Please address comments. Thanks,

@hansemannn
Copy link
Collaborator

@timanrebel Can you address the comments above? I'd merge afterwards. Thank you!

@hansemannn
Copy link
Collaborator

@timanrebel ping!

@timanrebel
Copy link
Contributor Author

Not to be rude, but I created this PR 1.5 years ago and do not intent to update it anymore. Please address the small changes yourself if you want to incorporate it in your product. It would have been nice to get a response sooner than almost a year later.

@hansemannn
Copy link
Collaborator

The PR is a feature request that did not have a priority at this time. That's why it was reviewed 1 year later (last November) with the comments to apply changes to the PR. Since you cannot address the comments, we will close it now.

@hansemannn hansemannn closed this Mar 23, 2016
@martinhudson
Copy link

Typically unhelpful response from Appcelerator
So much for supporting customer requests

@hansemannn
Copy link
Collaborator

@martinhudson I will describe it in detail to you: We have hundreds of tickets to look into every week. Prioritizing them takes it time, resolving them even more. This pull request does neither fixes a critical issue, nor has received much response by the community to be integrated. Yes, it took longer than usually to actually review the PR (in which we made good progress over the last 6 months), but if the contributor then is not able to adjust changes being proposed, we cannot accept a PR. And finally, as this is an open source project, everybody (including you) is welcomed to contribute to the project in order to improve it's functionality.


For me personally (aside my work for Appcelerator), I find it very inequitable to say we don't support our customers and community while discussing, merging and honoring almost 20 community PR's in the last 3 months.

@timanrebel
Copy link
Contributor Author

Generally response on both Jira and Github has been low to non-existent in the last few years. Do not know if it has improved lately.

You could have easily fixed indentation yourself and add the needed space, especially since the PR was filed 15 months ago. Instead of pingen me two times and then closing it because I don't want to setup the whole development environment again for 3 tabs and a space.

I must agree with @martinhudson... I was very disappointed by your response and I will also not file any future PR's. You could also have responded like @viezel did here: viezel/TiSocial.Framework#151 (comment)

@hansemannn
Copy link
Collaborator

Unlike in your PR in @viezel's repository, in this case you made very clear that you won't update anything. I still take the full responsibility in closing a PR that is battered by the initial contributor. If you watch other (much bigger repositories), PR's get closed without any comments just because they don't fit into the plans of the maintainer without modifications. That's not the way it should work and as said before, we are still trying to improve both Github- and JIRA communication. People like @falkolab and @m1ga did great contributions the last weeks and we appreciate any incoming PR while trying to fit it into our other projects which we work on. Instead of clarifying my decision here, I could continue on writing great utilities for the upcoming Hyperloop release, but that's another thing and fine like it is!

If you feel like you won't contribute anything because of this case, I am very sorry for your decision, but I stand behind the reasons I closed this PR. And if it heals your heart, I will will cherry-pick the commit and implement the review notes for you. Keep rocking! 🚀

@martinhudson
Copy link

Sir,
The company I work for is in fact an Enterprise customer, they pay for Appcelerator and I find it irritating that an item that needs fixing has been potentially fixed by a community member free-of-charge yet Appcelerator has in fact chosen to reject the change because of white-space. Appcelerator is a for-profit business, could have improved their product, but chose not to, providing little incentive for community members to continue to improve a great product.

I will raise a ticket and escalate through our Appcelerator account manager.

regards,
Martin

@hansemannn
Copy link
Collaborator

Thank you! In a nutshell: I did not see the watchers linked to the JIRA ticket when I closed this PR and was too fast closing it. Earlier PR's in the last months have been resolved by both our engineers and the community to help were we can. I apologize for the frustration caused for @timanrebel, you contributions have always been good and are still welcome, as well as every other contribution. Thanks guys! 😊

EDIT: The final PR's can be found here and here. We decided to also backport it to 5_3_X so the change is available asap. Thanks again!

@sgtcoolguy sgtcoolguy reopened this Apr 6, 2016
@ingo
Copy link
Contributor

ingo commented Apr 6, 2016

I spoke to Hans. He feels terrible about how this transpired. He's been doing a great job with other PRs. It's our fault for having taken this long to review a PR to begin with, and it it's on us to make it right if we screwed up. We'll pull this into a upcoming release ASAP. I'm sorry for any problems we caused, and we'll endeavor to do better in the future.

@sgtcoolguy
Copy link
Contributor

Ok, @hansemannn has re-opened another PR with this fix and the indentation/style fixes: #7920

@sgtcoolguy sgtcoolguy closed this Apr 6, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants