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

Lektor 613 - Passing extra_flags to all plugin events #651

Merged
merged 10 commits into from
May 13, 2019
Merged

Conversation

nixjdm
Copy link
Member

@nixjdm nixjdm commented May 12, 2019

Fixes #613.

This passes extra_flags to every plugin event, and allows extra_flags in a few more commands like clean and dev shell. This includes tests for most of the added functionality.

The tests could be improved, but the bigger deficiencies are not easy to overcome:

  • Testing the events hooked by lektor server; server_spawn and server_stop. Maybe there's an easy way I haven't thought of?
  • Including cli triggered hooking of the markdown config commands. Tough because of the markdown thread local.
  • Cleaning up after creating and installing plugins within the single pytest command. That's much more difficult than I originally realized. See inline comment block.

IMO, none of those are bad enough to block this. I'll happily accept recommended changes though if someone thinks of something I haven't :)

Ping @Andrew-Shay, for lektor/lektor-website#256.

Copy link
Contributor

@skorokithakis skorokithakis left a comment

Choose a reason for hiding this comment

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

This looks good to me at a first glance, but I'm not super duper familiar with the codebase.

@nixjdm
Copy link
Member Author

nixjdm commented May 12, 2019

Looks like I messed up the working directory pathing of the tests a bit. I'm fixing that.

Thanks for the look!

@nixjdm
Copy link
Member Author

nixjdm commented May 13, 2019

I think I'm done modifying this, tweaking the tests.

@goanpeca
Copy link
Member

Looks pretty good in general :-)

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.

Make extra flags more broadly available
3 participants