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.

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

Allow dependencies to be installed in 32 bits bottle #3058

Closed
wants to merge 4 commits into from

Conversation

Kinsteen
Copy link
Contributor

@Kinsteen Kinsteen commented Aug 25, 2023

Description

See #2861, couldn't update because I don't have write permissions anymore.

I didn't test, but you can look at the previous PR to know what it does and how to test it.

Also, an update to the .pot files should be required to have the new string properly translated.

@github-actions
Copy link
Contributor

Pylint result on modfied files:
************* Module bottles.frontend.views.bottle_dependencies
bottles/frontend/views/bottle_dependencies.py:67:61: C0303: Trailing whitespace (trailing-whitespace)
bottles/frontend/views/bottle_dependencies.py:21:0: E0611: No name 'Adw' in module 'gi.repository' (no-name-in-module)
bottles/frontend/views/bottle_dependencies.py:87:12: C0103: Variable name "r" doesn't conform to snake_case naming style (invalid-name)
************* Module bottles.frontend.widgets.dependency
bottles/frontend/widgets/dependency.py:22:0: E0611: No name 'Adw' in module 'gi.repository' (no-name-in-module)
bottles/frontend/widgets/dependency.py:59:12: W0105: String statement has no effect (pointless-string-statement)
bottles/frontend/widgets/dependency.py:90:12: W0105: String statement has no effect (pointless-string-statement)
bottles/frontend/widgets/dependency.py:99:12: W0105: String statement has no effect (pointless-string-statement)
bottles/frontend/widgets/dependency.py:166:49: W0613: Unused argument 'error' (unused-argument)

@koplo199
Copy link
Contributor

koplo199 commented Sep 6, 2023

@orowith2os Any chance to get this PR merged? Seems the previous one was approved: #2861 (comment)

@orowith2os
Copy link
Contributor

@koplo199 needs testing, otherwise I can look at merging it.

@fab-sonarqube
Copy link

fab-sonarqube bot commented Sep 6, 2023

if self.manager.utils_conn.status == False:
self.stack.set_visible_child_name("page_offline")
if not self.manager.utils_conn.status:
self.stack.set_visible_child_name("page_offline")
Copy link
Contributor

@koplo199 koplo199 Sep 26, 2023

Choose a reason for hiding this comment

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

Suggested change
self.stack.set_visible_child_name("page_offline")
self.stack.set_visible_child_name("page_offline")

Remove space at end of line

@koplo199
Copy link
Contributor

koplo199 commented Sep 26, 2023

Seems the part handling the for verb from the previous PR is missing:

arch = step.get("for", "win64_win32")
if config.Arch not in arch:
continue

(Apparently GitHub does not allow suggesting change to unmodified files/lines, so I cannot leave a suggestion there)

@koplo199
Copy link
Contributor

koplo199 commented Sep 26, 2023

It also seems the remove_dependency did not benefit from the widget to _widget renaming:

def remove_dependency(self, widget):
"""
This function remove the dependency from the bottle
configuration
"""
widget.set_sensitive(False)

I also can't leave a change suggestion, for the same reason.

@koplo199
Copy link
Contributor

Fixes #2918
Fixes #2166

@koplo199
Copy link
Contributor

@Kinsteen Sorry to ping you directly, I assume you've been busy in life lately.

Hope you don't mind if I implement the requested changes above so we could merge soon. I'll do so by forking your branch 32bits-dependencies, meaning you'll of course retain full authorship of the commits present in there.

@Kinsteen
Copy link
Contributor Author

@koplo199 I have dropped working on Bottles, I would gladly appreciate if you could do the modifications :)

@orowith2os
Copy link
Contributor

Closing in favor of #3101

@orowith2os orowith2os closed this Sep 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants