Skip to content

Conversation

@reegnz
Copy link
Contributor

@reegnz reegnz commented May 8, 2025

python peps are enhancement proposal documents, they are usually
referred to as PEP-xxx, PEP xxx or PEPxxx, the handler handles each of
those cases.

Signed-off-by: Zoltán Reegn [email protected]

python peps are enhancement proposal documents, they are usually
referred to as PEP-xxx, PEP xxx or PEPxxx, the handler handles each of
those cases.

Signed-off-by: Zoltán Reegn <[email protected]>
@reegnz
Copy link
Contributor Author

reegnz commented May 8, 2025

I'll add some tests to this PR a bit later, just wanted to feel around if this would be a good candidate for this plugin.

@chrishrb
Copy link
Owner

chrishrb commented Jul 6, 2025

can you also add some tests here?

@reegnz
Copy link
Contributor Author

reegnz commented Jul 7, 2025

Thanks for pinging, I totally forgot about this PR! 😅

Added a test.

@chrishrb
Copy link
Owner

chrishrb commented Jul 7, 2025

you also have to add the handler in the handler.lua file. here is also the priority set, so it makes sense to add it above or below the cve_handler?

@reegnz
Copy link
Contributor Author

reegnz commented Jul 8, 2025

Whoops, turns out as I was testing it I had it as a custom handler so I didn't notice it not getting registered.
Added the handler registering code now.
Maybe some integration test would catch that.
eg. not directly loading the handler code in tests but rather loading the entire plugin, exercising the gx mapping and capturing the output with an open_callback, and also make the open_browser_app a NOOP during testing.

I can see if I can improve that when I have some time, what do you think @chrishrb?

Strictly in a follow-up PR 😅

@chrishrb
Copy link
Owner

chrishrb commented Jul 8, 2025

yes, of course. that would be super cool, tests are always welcome (and super important!) 👍

@chrishrb chrishrb merged commit d730e1f into chrishrb:main Jul 8, 2025
2 checks passed
@reegnz reegnz deleted the add_python_pep_handler branch July 8, 2025 09:19
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.

2 participants