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

Library: Port 'Text Colors' to Python #735

Merged
merged 2 commits into from
Nov 11, 2023
Merged

Library: Port 'Text Colors' to Python #735

merged 2 commits into from
Nov 11, 2023

Conversation

UrtsiSantsi
Copy link
Contributor

No description provided.

@theCapypara
Copy link
Contributor

Very sorry about the CI issues, that was another demo that caused this. Will be fixed by #736, you may want to update your branches after this.
With all these many PRs one slipped through that had bad formatting.

@UrtsiSantsi
Copy link
Contributor Author

I don't get what I should do - should I rebase on top of "main" or get "main" and use it to reformat it?

@UrtsiSantsi UrtsiSantsi mentioned this pull request Oct 31, 2023
Closed
@UrtsiSantsi
Copy link
Contributor Author

Fixed

theCapypara
theCapypara requested changes Nov 3, 2023
View reviewed changes
src/Library/demos/Text Colors/main.py
import gi

gi.require_version("Gtk", "4.0")
gi.require_version("Pango", "1.0")
Copy link
Contributor

Choose a reason for hiding this comment

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

Specifying the version for Pango is not necessary, there is only one version.

Copy link
Contributor Author

@UrtsiSantsi UrtsiSantsi Nov 3, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Only one so far 😄 - there is a pango2 pull request by Matthias Clasen. Is this a problem in reality, specifying a version even if there is only one or just a personal preference?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

And what about Glib and Gio?

Copy link
Contributor

@theCapypara theCapypara Nov 3, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Oh sorry, I wasn't aware Pango 2 was a thing. In that case we should keep this 👍
In general my comment here: #732 (comment)

Copy link
Contributor

Choose a reason for hiding this comment

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

See also #738 - I'll think about / do some research if specifying the Pango version makes sense, but in doubt we keep it.

theCapypara
theCapypara approved these changes Nov 11, 2023
View reviewed changes
Copy link
Contributor

@theCapypara theCapypara left a comment

Choose a reason for hiding this comment

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

LGTM! There were a few minor code styling issues, I added a commit for those.

There was also a debug print still in there, removed that as well.

@theCapypara theCapypara merged commit e42c32e into main Nov 11, 2023
1 check passed
@theCapypara theCapypara deleted the python_text_colors branch November 11, 2023 16:37
lw64 pushed a commit that referenced this pull request Dec 18, 2023
@UrtsiSantsi @theCapypara @lw64
Library: Port 'Text Colors' to Python (#735)
62256bd 
Co-authored-by: Marco Capypara Köpcke <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@theCapypara theCapypara theCapypara approved these changes

Assignees
No one assigned
Labels
Library 📚️ python
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@UrtsiSantsi @theCapypara