-
Notifications
You must be signed in to change notification settings - Fork 25
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 to run the module without installing it #138
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm interesting, not possible for me to try out myself atm but wouldn't that work already:
python -m synadm.cli
but that would require it to be installed already... the debian package requires it to be able to run without installing? why? during packaging? |
Not the Debian package directly, but the way Click based CLI's shell completion work is by calling the program with a specific environment variable set to generate the completion script. I make this call in Line 4 of debian/rules, and I didn't find any better way to make it than by making the package executable with But if you don't want this change, it can also stay a Debian only patch, as it is very small and should not change too often. And if you have better suggestions I am also interested. |
Nope that doesn't work :-)
|
Hi @n-peugnet, I'm not opposed to this change at all. Please let's add it. Some things I'd like to add within this PR though: Some docstrings in the new module Also I would like to add to the docs, the new possibility to run synadm programatically, it might be of interest to other packagers or to people who'd like to integrate synadm into their own programs. Not sure where in the docs I would see that located though... And also, but this may be off-topic here, but I would like to include it in the PR while we are at it and we have the expert at hand already (you ;-), let's add to the docs what commands and setup would be required to make synadm bash completion work when synadm is installed via the pip package. Maybe have the concrete commands for bash and zsh right in our docs but as well linking to the Click docs: https://click.palletsprojects.com/en/8.1.x/shell-completion/ If you find time to help us with these things within this PR that would be wonderful. Certainly not urgent since we are not fixing a bug here but at some point this or next year? |
I realized where in the docs the main module could be included. Since it is very much only interesting for developers, it would fit as a new page in the "package documentation" section. Creating a file named
The docstring you add to the source would then automatically get rendered within our docs. Finally adding a line
synadm/doc/source/index_modules.rst Lines 8 to 9 in ea1b1e0
and it should be accessible in the TOC. You can build the docs locally (but I see that a howto for that is missing in our docs, I should add that) or tell me to check out your branch and I'll test it. |
update again: @n-peugnet I'm not 100% sure about my previous post's description #138 (comment) anymore. Maybe forget about it for now and just add the docstring in |
Hi @n-peugnet, I just wanted to the following commit to this PR to finalize it: 4e41cd5 It seems I'm not allowed to push that commit to your branch. Would it be possible for you to allow "maintainer changes" on your end? https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/allowing-changes-to-a-pull-request-branch-created-from-a-fork#enabling-repository-maintainer-permissions-on-existing-pull-requests What I did is adding you as the author of the file and I elaborated on why this file is here. Please review and tell me if you would agree with these changes. Thanks! |
This is perfect ! Sorry, I didn't answer you previous message, thank you for doing this yourself.
Well, this is strange, I don't have this checkbox on this pull request page. It might be because I forked the repo in an organisation. I think I would need to create a new pull request and close this one. But if you are OK to simply merge 4e41cd5 into main, this is fine by me. |
I figured I could simply pull your commit in my branch. |
Yeah a cherry-pick would do it. Thanks! |
Already done, in case you didn't see |
Ah yeah missed it. Many thanks! |
As explained previously, I want to add the click generated bash completion script to the Debian package. Here is the change I sent to the Debian maintainer of synadm : https://salsa.debian.org/matrix-team/synadm/-/merge_requests/1
The only way I found to make it work was to add a "main" file that allows to run synadm without installing it. As I thought it could be useful outside the Debian package, here is the forwarded patch, that allows to run synadm directly with
python -m synadm
.I am no expert in Python so this might not be the ideal way to do it.