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

Fix setup #126

Merged
merged 1 commit into from May 11, 2023
Merged

Fix setup #126

merged 1 commit into from May 11, 2023

Conversation

sanzoghenzo
Copy link
Contributor

This will hopefully fix the installation issue reported in #125.

The remote debugger was enabled, I added an environment variable to avoid rogue commits (and updated the readme accordingly).

I also took the liberty to make things more pythonic (mostly PEP8 naming style) and make the setup code DRY.

@sanzoghenzo
Copy link
Contributor Author

sanzoghenzo commented May 8, 2023

Sorry, I see only now that you don't support the linux platform.

The PR solves the installation/activation problem, but I only tested on linux and then I got stuck because no accounts can be detected (no manager to read the info from).

EDIT: I followed the documentation on how to add an account manually and it works, so I guess linux can be added to the supported platforms along with a link to the documentation to add the accounts

@KatKatKateryna
Copy link
Contributor

KatKatKateryna commented May 9, 2023

Hi @sanzoghenzo ! Glad it worked in the end.
Thanks for the contribution! There are some changes already piled up, I will take a look at PR right after they are merged, this or next week. Would you mind sharing which method you followed to make the accounts work on linux?

@sanzoghenzo
Copy link
Contributor Author

Sorry, I thought I posted it already, it may be lost in my many edits 😅

Here it is

@KatKatKateryna
Copy link
Contributor

Hi @sanzoghenzo ! The extra "bin" is necessary for Mac, as the executable path is not detected correctly (like reported, for example, here).

Could you please create another PR with only one check for mac/linux os in the existing get_qgis_python_path function, without any extra restructuring? Then I can merge it immediately and release this week with a new version. Remote debugging and dependencies are currently undergoing changes too.

Thanks!

@sanzoghenzo
Copy link
Contributor Author

sanzoghenzo commented May 11, 2023

Hi, I hope it's OK that I just force-pushed the branch of this PR with what you asked.

I'm looking forward to see the new changes you have in store!

@KatKatKateryna
Copy link
Contributor

KatKatKateryna commented May 11, 2023

Awesome, thanks!
More details on the same issue for the reference: qgis/QGIS#45646

@KatKatKateryna KatKatKateryna merged commit 931925c into specklesystems:main May 11, 2023
1 check passed
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.

None yet

2 participants