Skip to content
This repository was archived by the owner on Sep 6, 2021. It is now read-only.

Now reverse inspect can be disabled by setting the 'livedev.enableReverseInspect' pref to false #13659

Merged
swmitra merged 2 commits intomasterfrom
saurabh95/switch-reverse-inspect-on-off
Sep 18, 2017
Merged

Now reverse inspect can be disabled by setting the 'livedev.enableReverseInspect' pref to false #13659
swmitra merged 2 commits intomasterfrom
saurabh95/switch-reverse-inspect-on-off

Conversation

@saurabh95
Copy link
Copy Markdown
Contributor

@saurabh95 saurabh95 commented Sep 4, 2017

Fixes: #13307

Comment thread src/LiveDevelopment/LiveDevelopment.js Outdated
prepareServerPromise
.done(function () {
WebSocketTransport.createWebSocketServer(PreferencesManager.get("livedev.wsPort"));
var wsPort = PreferencesManager.get("livedev.wsPort");
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Any reason not to use a different pref instead, such as livedef.enableReverseInspect

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@petetnt IMO, that might confuse a lot of users. Instead if an invalid port (0) is given by the user, it's implicit to assume that reverse inspect is not required.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

My 2 cents is that average user doesn't even know what a port is, even without considering what might be an invalid port and what isn't 🤔

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I am a pendulum now 😃 . I had asked the same initially to @saurabh95 but somehow got convinced with the explanation. But now I am convinced by your explanation again so would ask the author to include a new preference. @saurabh95 over to you...

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yeah, for a user who is not aware of ports, it would be a difficult way to disable reverse inspect. I will add a new preference and will update this PR only.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

👍

@swmitra
Copy link
Copy Markdown
Collaborator

swmitra commented Sep 18, 2017

@petetnt Can you please have a look at the new pref? I have to merge this PR today and get the localized description strings as well.

@swmitra swmitra merged commit 2da7ddb into master Sep 18, 2017
@swmitra
Copy link
Copy Markdown
Collaborator

swmitra commented Sep 18, 2017

Great work @saurabh95 and @petetnt 👍

@swmitra swmitra deleted the saurabh95/switch-reverse-inspect-on-off branch September 18, 2017 08:24
@swmitra swmitra changed the title Now reverse inspect can be disabled by setting the livedev.wsPort to 0 Now reverse inspect can be disabled by setting the 'livedev.enableReverseInspect' pref to false Sep 18, 2017
@swmitra swmitra added this to the Release 1.11 milestone Sep 22, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants