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.

Sign up for GitHub

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

Jump to bottom

Refactor code in background.js #12

Open
vinitshahdeo opened this issue Oct 10, 2019 · 12 comments
Open

Refactor code in background.js #12

vinitshahdeo opened this issue Oct 10, 2019 · 12 comments
Labels
good first issue Good for newcomers hacktoberfest help wanted Extra attention is needed up-for-grabs Solve the issue and raise your first PR

Comments

@vinitshahdeo
Copy link
Owner

  • Please check background.js and refactor the different methods inside this.

  • Do small, incremental changes that leave the code in a better state than it was found.

  • Do not make any changes in other files.

  • Make sure that the existing functionality shouldn't break.
@vinitshahdeo vinitshahdeo self-assigned this Oct 10, 2019
@vinitshahdeo vinitshahdeo added good first issue Good for newcomers hacktoberfest help wanted Extra attention is needed up-for-grabs Solve the issue and raise your first PR labels Oct 10, 2019
@vinitshahdeo vinitshahdeo removed their assignment Oct 10, 2019
@aleoof
Copy link

aleoof commented Oct 10, 2019

i will do this

@JordanRosas
Copy link

Is anyone assigned to this issue? I would like to participate if possible!

@vinitshahdeo
Copy link
Owner Author

@aleoof Yes, please go ahead.

@vinitshahdeo
Copy link
Owner Author

@JordanRosas No actually, give it a shot, I'll merge the most optimized code.

@JordanRosas
Copy link

JordanRosas commented Oct 11, 2019 •
edited
Loading

Hi @vinitshahdeo last night I ran your project fresh and and when I opened my devtools I got three errors

  • A bad HTTP response code (404) was received when fetching the script.
  • background.js:174 Uncaught TypeError: Cannot read property 'get' of undefined
    at setCookieCount (background.js:174)
    at background.js:180 (Don't mind the line numbers I had my refactored code sitting on top of the old code for reference)
  • GET http://127.0.0.1:8080/index.js net::ERR_ABORTED 404 (Not Found)

is there something I am missing on my end?

@vinitshahdeo
Copy link
Owner Author

@JordanRosas It seems you opened index.html in chrome directly. It's a chrome extension so it's supposed to be installed locally. Please refer this.

@JordanRosas
Copy link

JordanRosas commented Oct 11, 2019 via email

@vinitshahdeo
Copy link
Owner Author

@JordanRosas My bad. Please delete that file (_config.yml). It's not supposed to be there. This for GitHub pages.

@JordanRosas
Copy link

JordanRosas commented Oct 11, 2019 via email

@vinitshahdeo
Copy link
Owner Author

@JordanRosas You're welcome!

@JordanRosas
Copy link

@vinitshahdeo I have a PR up for some reason I couldnt add you as a reviewer?

@vinitshahdeo
Copy link
Owner Author

@JordanRosas No problem. Thank you for the PR. I'll review and merge it soon.

You may look into a few more good first issues here which are up for grabs!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
good first issue Good for newcomers hacktoberfest help wanted Extra attention is needed up-for-grabs Solve the issue and raise your first PR
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@vinitshahdeo @aleoof @JordanRosas