Skip to content
This repository has been archived by the owner on Jul 31, 2019. It is now read-only.

disable-lint #2024

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

sgupta7857
Copy link

0.4 adding the toggle button for linting

delete req.session.home;
return res.redirect(301, path.join("/", locale));
}

Copy link
Contributor

Choose a reason for hiding this comment

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

Revert this change please, git checkout master server/routes/auth/oauth2-callback.js

var $allowWSToggle = $("#allow-lint-toggle");
var toggle = !($allowWSToggle.hasClass("switch-enabled"));

if(toggle) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Indenting issues here too. In Thimble we use 2-spaces. Your entire if block is indented 2-spaces too far.

@@ -78,7 +78,24 @@ define(function(require) {

return false;
});
$("#allow-lint-toggle").click(function() {
// Toggle current value
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove this.

@gideonthomas
Copy link
Contributor

@sgupta7857 please reference the issue # this is fixing in the PR description.

@sgupta7857
Copy link
Author

@humphd I made the changes on thimble side as you suggested. But the toggle button is not working for some reason. can you please guide what I am doing wrong here?

@sgupta7857
Copy link
Author

this refers to the issue https://github.com/mozilla/thimble.mozilla.org/issues/1896

@gideonthomas
Copy link
Contributor

gideonthomas commented Apr 28, 2017

@sgupta7857 you removed all your changes to bramble-menus.js which sets up the event listeners needed to get the toggle working.

You'll also want to rebase your patch onto mozilla/master

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