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

Incorrect error thrown when function is in comments #10929

Merged
merged 1 commit into from Oct 16, 2016
Merged

Incorrect error thrown when function is in comments #10929

merged 1 commit into from Oct 16, 2016

Conversation

Bouncey
Copy link
Member

@Bouncey Bouncey commented Sep 26, 2016

Pre-Submission Checklist

  • Your pull request targets the staging branch of FreeCodeCamp.
  • Branch starts with either fix/, feature/, or translate/ (e.g. fix/signin-issue)
  • You have only one commit (if not, squash them into one commit).
  • All new and existing tests pass the command npm run test-challenges. Use git commit --amend to amend any fixes.

Type of Change

  • Small bug fix (non-breaking change which fixes an issue)

Checklist:

Description

Added new regExp in detect-unsafe-code-stream.js to detect if the word function is in comments and if so, ignore and do not throw the error SyntaxError: Unsafe or unfinished function declaration

@BerkeleyTrue BerkeleyTrue added the status: waiting review To be applied to PR's that are ready for QA, especially when additional review is pending. label Sep 26, 2016
@ktajpuri
Copy link
Contributor

@Bouncey could you please explain the regex you used? A bit tough to figure out for me.

@Bouncey
Copy link
Member Author

Bouncey commented Oct 12, 2016

cc @FreeCodeCamp/issue-moderators - this is ready

Copy link
Member

@ltegman ltegman left a comment

Choose a reason for hiding this comment

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

This looks good to me, but I can't get the branch up and running locally for some reason. Would love a confirmation from someone who can get it running that this works as it should.

@ghost
Copy link

ghost commented Oct 13, 2016

@ltegman, I will test this in a few hours 👍

@ghost
Copy link

ghost commented Oct 13, 2016

Having issue with the tests on my local setup. Don't believe it's an issue with this PR however.

cc/ @FreeCodeCamp/issue-moderators, please try and review this PR!

@Bouncey
Copy link
Member Author

Bouncey commented Oct 14, 2016

Unable to QA due to #11198

@ltegman
Copy link
Member

ltegman commented Oct 16, 2016

Something about the state of staging when you made your changes is what was causing problems for me. I rebased this branch against the latest staging and was able to QA just fine. LGTM 👍

@ltegman ltegman merged commit 96f5b3a into freeCodeCamp:staging Oct 16, 2016
@ltegman ltegman removed the status: waiting review To be applied to PR's that are ready for QA, especially when additional review is pending. label Oct 16, 2016
@BerkeleyTrue
Copy link
Contributor

@FreeCodeCamp/issue-moderators

When merging a PR please ensure the PR actually works or does what it says. This file is no longer used anywhere and does not run actually run anywhere.

@ltegman
Copy link
Member

ltegman commented Oct 25, 2016

Huh. I compared this code side by side with code that didn't have the change and saw an improvement, but it must have been something else doing it. Sorry about that!

@Bouncey Bouncey deleted the fix/functionInComments branch October 31, 2016 22:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants