Skip to content

PR: Test using PyQt extra packages#446

Merged
ccordoba12 merged 17 commits intospyder-ide:masterfrom
dalthviz:fix_issue_383
Aug 28, 2023
Merged

PR: Test using PyQt extra packages#446
ccordoba12 merged 17 commits intospyder-ide:masterfrom
dalthviz:fix_issue_383

Conversation

@dalthviz
Copy link
Copy Markdown
Member

@dalthviz dalthviz commented Aug 18, 2023 •
edited
Loading

  • Add CI matrix value to install PyQt extra packages
  • Remove setting specific QScintilla version for PyQt6 and instead just install it without version constraint when using the pyqt-extras flag: checking the CI logs the test was being skipped for errors when trying to import QSci due to DLL errors or because the specified version was not compatible with the PyQt6 version being installed and then the import failed.
  • Add step to review jobs matrix/config values
  • Move some jobs to run using latest Qt6 bindings (6.5)
  • Update README with testing matrix value section and some improvements to the typing section and the general content

Fixes #383

@dalthviz dalthviz added this to the v2.4.0 milestone Aug 18, 2023
@dalthviz dalthviz self-assigned this Aug 18, 2023
@dalthviz dalthviz force-pushed the fix_issue_383 branch 2 times, most recently from 4372309 to a0fa096 Compare August 25, 2023 17:11
@dalthviz dalthviz changed the title [WIP] PR: Test using PyQt extra packages PR: Test using PyQt extra packages Aug 25, 2023
@dalthviz dalthviz marked this pull request as ready for review August 25, 2023 18:42
ccordoba12
ccordoba12 reviewed Aug 25, 2023
View reviewed changes
Copy link
Copy Markdown
Member

@ccordoba12 ccordoba12 left a comment

Choose a reason for hiding this comment

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

Thanks @dalthviz! Some small suggestions to improve readability and wording, the rest looks good to me.

@dalthviz
Copy link
Copy Markdown
Member Author

dalthviz commented Aug 25, 2023 •
edited
Loading

Thanks for the review @ccordoba12 ! As a side note, probably it could be better to wait for #451 to get merged before merging this, right?

ccordoba12
ccordoba12 reviewed Aug 25, 2023
View reviewed changes
Copy link
Copy Markdown
Member

@ccordoba12 ccordoba12 left a comment

Choose a reason for hiding this comment

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

Last suggestions, then this should ready for merge.

@ccordoba12
Copy link
Copy Markdown
Member

ccordoba12 commented Aug 25, 2023

As a side note, probably it could be better to wait for #451 to get merged before merging this, right?

#451 is going to touch a lot of things, so it could be difficult to merge this after that one.

@Czaki, what do you think?

@dalthviz @ccordoba12
Apply suggestions from code review
8515434 
Co-authored-by: Carlos Cordoba <ccordoba12@gmail.com>
@Czaki
Copy link
Copy Markdown
Contributor

Czaki commented Aug 25, 2023

At this moment, I do not see conflicts between these two packages. Is some happen (formating) then execute black should easily solve them.

ccordoba12
ccordoba12 approved these changes Aug 28, 2023
View reviewed changes
Copy link
Copy Markdown
Member

@ccordoba12 ccordoba12 left a comment

Choose a reason for hiding this comment

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

Thanks @dalthviz!

@ccordoba12 ccordoba12 merged commit d7ff724 into spyder-ide:master Aug 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ccordoba12 ccordoba12 ccordoba12 approved these changes

Assignees

@dalthviz dalthviz

Labels

type:Task

Projects

None yet

Milestone

v2.4.0

Development

Successfully merging this pull request may close these issues.

Add extra packages available for PyQt to CI for testing

3 participants

@dalthviz @ccordoba12 @Czaki