Skip to content

Update native-messaging to use python3, clarify instructions #564

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

Merged
merged 8 commits into from
Oct 23, 2024

Conversation

dotproto
Copy link
Contributor

@dotproto dotproto commented Jul 13, 2024

Description

The following updates were made to the native-messaging example.

  • Change the shebang in app/ping_pong.py explicitly use Python 3 instead of Python 2
  • Change the app/ping_pong_win.bat runner to use Python 3 (untested)
  • Updated the README
    • with more details on how to set up Python 3
    • explicitly tell the user to update files in app/ with the full paths of the files they reference
    • remove the Python 2 in the README as support for Python 2 ended in 2020
  • Added a port.onDisconnect listener to add-on/background.js

Motivation

Update the example to function as expected.

Additional details

N/A

Related issues and pull requests

Fixes #560

Copy link

It looks like this is your first pull request. 🎉 Thank you for your contribution! One of the project maintainers will triage and assign the pull request for review. We appreciate your patience. To safeguard the health of the project, please take a moment to read our code of conduct.

@dotproto dotproto requested review from rebloor and willdurand July 13, 2024 02:06
@dotproto
Copy link
Contributor Author

@rebloor, at your suggestion I took a look at the native-messaging example and also wasn't able to get it to run locally on my test device using the instructions provided. Please take a look at the updated instructions and revise as appropriate.

@rebloor
Copy link
Collaborator

rebloor commented Jul 13, 2024

Thanks @dotproto I'll take a look first thing next week

@dotproto dotproto requested a review from Rob--W July 17, 2024 19:41
Copy link
Collaborator

@rebloor rebloor left a comment

Choose a reason for hiding this comment

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

Some suggestions for your consideration. I'll test the code tomorrow.

Co-authored-by: rebloor <git@sherpa.co.nz>
@github-actions github-actions bot added the idle Issues and pull requests with no activity for three months. label Aug 18, 2024
@github-actions github-actions bot removed the idle Issues and pull requests with no activity for three months. label Oct 19, 2024
Example now simply returns "pong"
Copy link
Collaborator

@rebloor rebloor left a comment

Choose a reason for hiding this comment

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

Tested on macOS - all OK

@dotproto dotproto merged commit 18238c1 into mdn:main Oct 23, 2024
3 checks passed
Copy link

Congratulations on your first merged pull request. 🎉 Thank you for your contribution! Did you know we have a project board with high-impact contribution opportunities? We look forward to your next contribution.

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.

native messaging example: port disconnects without an error
3 participants