search

Bhupesh-V / tutorialdb

A search 🔎 engine for programming/dev tutorials,
MIT License
121 stars 60 forks source link

Remove run error on focusMe #46

Closed liorbentov closed 4 years ago

Animesh-Ghosh commented 4 years ago

Please create an issue first before creating a Pull Request.

It helps in associating PRs with the issues that they are solving.

Animesh-Ghosh commented 4 years ago

A description would have also sufficed. Creating an issue for errors may be a good idea, but creating issues for increasing readability might be a bit much.

Please update #49 accordingly. I wasn't thinking as well.

Animesh-Ghosh commented 4 years ago

This PR solves what run error exactly? Is it the ConnectionAbortedError? Please add a description about what exactly this change solves.

If it's a small issue (ex. readabilty), then only description may suffice. However, if it's a major issue creating an Issue would help document what changed and why.

liorbentov commented 4 years ago

Sorry for that, it has been a long time since I've posted a PR... When entering the About page, there is a console error that is caused by focusMe implementation.

Animesh-Ghosh commented 4 years ago

You meant the browser console? That took me a while to figure out.

While it does get rid of the TypeError, a ReferenceError comes up in the Homepage when one searches for tutorials.

EDIT: Do update custom.js to match with master, there's a conflict.

liorbentov commented 4 years ago

@Animesh-Ghosh, fixed

Animesh-Ghosh commented 4 years ago

Fixing TypeError introduces a ReferenceError.

liorbentov commented 4 years ago

Where? What are the steps to reproduce?

Animesh-Ghosh commented 4 years ago

Sorry, it seems removing the TypeError doesn't introduce the ReferenceError, it already was there before.

Steps to reproduce:

  1. Run server.
  2. Search for tutorial.
  3. Let the page load.
  4. Check browser console.
liorbentov commented 4 years ago

@Animesh-Ghosh , I think it's right to open an issue/bug for that if it's not related. What do you say?

Animesh-Ghosh commented 4 years ago

Really need to update the CONTRIBUTING.md file. 😅

liorbentov commented 4 years ago

Really need to update the CONTRIBUTING.md file. 😅

I don't think that's related as well 🤷‍♀ What do you want this file to say? Does this PR resolve the error? Does it create a new one? If it resolves the error and doesn't create a new one, I think it's right to merge it and stop the discussion, or at least move it to somewhere proper.

liorbentov commented 4 years ago

@Animesh-Ghosh , I've opened a PR (#54 ) for fixing the reference error.

Bhupesh-V commented 4 years ago

@liorbentov should i merge this one or #54 ?

liorbentov commented 4 years ago

First this one, and then I will update #54