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

Add docs for py-event* #1300

Merged
merged 7 commits into from
Mar 23, 2023
Merged

Add docs for py-event* #1300

merged 7 commits into from
Mar 23, 2023

Conversation

marimeireles
Copy link
Member

@marimeireles marimeireles commented Mar 22, 2023 •
edited
Loading

Description

Please describe the changes in your pull request in few words here.

Changes

List the changes done to fix a bug or introducing a new feature.

Checklist

  • All tests pass locally
  • I have updated docs/changelog.md
  • I have created documentation for this(if applicable)

@marimeireles marimeireles force-pushed the dj-fiorex/main branch 3 times, most recently from a611f39 to 5a0123b Compare March 22, 2023 13:20
@marimeireles
Copy link
Member Author

@dj-fiorex took the liberty of rebasing and attending to Jeff and Antonio's PR here. Hope it's not an issue. Thank you for the work here!
@antocuni can you please check if this is fine? I've rewrote the sentence you commented on and remove the duplication.
Cheers folks.

@marimeireles
Copy link
Member Author

closes #835

@marimeireles
Copy link
Member Author

BTW, I've built it locally and it shows the changes fine. 👍

@antocuni
Copy link
Contributor

@dj-fiorex took the liberty of rebasing and attending to Jeff and Antonio's PR here. Hope it's not an issue. Thank you for the work here!

which PR?

Moreover, this PR should have a more descriptive title, because as it stands I have no idea of what it is supposed to do.

@marimeireles
Copy link
Member Author

@antocuni sorry should have given more context.
The related issue is tagged above and the original PR is #852 you've reviewed months ago.
It's still relevant, imo, as long as I don't merge my PR on the new event handling stuff.

@marimeireles marimeireles changed the title Dj fiorex/main Add docs for py-event* Mar 23, 2023
fpliger
fpliger reviewed Mar 23, 2023
View reviewed changes
docs/guides/event-handlers.md
```python
<py-script>
def say_hello_no_param():
print("Hello!")
Copy link
Contributor

Choose a reason for hiding this comment

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

very minor but would be goo to add a new line between the 2 functions definition

Copy link
Member Author

Choose a reason for hiding this comment

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

thanks!
Will fix so we can merge it :)

fpliger
fpliger approved these changes Mar 23, 2023
View reviewed changes
Copy link
Contributor

@fpliger fpliger left a comment

Choose a reason for hiding this comment

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

Thanks @marimeireles @dj-fiorex , this is overall a good PR. It introduces a few sections that are Pyodide specific and we'll have to change/reshape once we better support MicroPython and add the new Workers + FFI interface.

👍

@marimeireles marimeireles merged commit 543a272 into pyscript:main Mar 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fpliger fpliger fpliger approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@marimeireles @antocuni @fpliger @dj-fiorex