spyder-ide / qtpy

Provides an uniform layer to support PyQt5, PySide2, PyQt6, PySide6 with a single codebase
MIT License
987 stars 153 forks source link

PR: Fix bug when importing from `PySide6.QtWebEngineCore/QtWebEngineWidgets` (`QWebEngineScrip` vs `QWebEngineScript`) #455

Closed damonlynch closed 1 year ago

damonlynch commented 1 year ago

Fix typo in import statement

damonlynch commented 1 year ago

Thanks @dalthviz, I have made the changes to the test code, which were pretty simple. However, my knowledge of git is quite minimal. Should I make a new, separate commit with the changes, or use some other kind of git command to incorporate the changes? Then how do I incorporate the changes into this pull request?

By the way, I was unable to understand how to set up pytest using the instructions in CONTRIBUTING.md. I ran the command pre-commit install, but I have no idea if that installed pytest.

dalthviz commented 1 year ago

However, my knowledge of git is quite minimal. Should I make a new, separate commit with the changes, or use some other kind of git command to incorporate the changes? Then how do I incorporate the changes into this pull request?

No problem! So, you will need to do the test change and commit that change over the fork branch that you named PySide6-QWebEngineScript-fix (that's the branch on your fork connected to this pull request). For that, if you already cloned your fork repo in your machine, run from a terminal or git bash instance something like:

cd <path where your fork clone is located>
git checkout PySide6-QWebEngineScript-fix

Then, update the relevant test file, add the changes, create a commit and push it. After doing the change to the test file, you will need to run something like:

git add .
git commit -m "Update QtWebEngineCore test"
git push

If that sounds a little bit to complicated, I think you can use the GitHub web interface to create the commit for the test too. For that, you just need to navigate to the file while having the related fork branch to this PR selected. In this case, I think you should be able to edit using the GitHub web interface using this link: https://github.com/damonlynch/qtpy/edit/PySide6-QWebEngineScript-fix/qtpy/tests/test_qtwebenginewidgets.py

By the way, I was unable to understand how to set up pytest using the instructions in CONTRIBUTING.md. I ran the command pre-commit install, but I have no idea if that installed pytest.

pytest is installed when you run the command in this section: https://github.com/spyder-ide/qtpy/blob/master/CONTRIBUTING.md#install-qtpy-in-editable-mode

And to run pytest you can follow the info in this section: https://github.com/spyder-ide/qtpy/blob/master/CONTRIBUTING.md#running-the-tests

The pre-commit setup is meant to be added to run some code style rules when you commit changes so is not related with pytest but maybe the phrasing in that section can be confusing now that I'm rereading it 🤔 Thank you for the feedback on that!

If you have further questions, regarding how to add the changes to this PR, comments or want for us to help you finish this let us know!

damonlynch commented 1 year ago

I didn't realise it was as easy as simply adding another commit. I have now done that, and pushed it to GitHub.

I also got confused by the section that explains how to install qtpy in editable mode, so I skipped that part. That means I have not personally run the changes I made to the test code in the most recent commit. However, given these changes are so simple, it probably doesn't matter in this instance.