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

Project-specific releaser hooks in setup.cfg #17

Closed
wants to merge 5 commits into from

Conversation

embray
Copy link
Contributor

@embray embray commented Jul 10, 2012

This pull request adds a feature I've been wanting a long time which is the ability to list project-specific releaser hooks in the setup.cfg for a project under the [zest.releaser] section.

Each hook point (prereleaser.before, etc) is given as one or more fully qualified Python function names. Just as with the entry_points, each hook function takes the data dict as its sole argument. entry_points are still run, but only after the project-specific hooks. This eliminates the annoyance of having to install my project just so that the correct entry_points are installed in order to do a release.

No tests yet, but if this approach looks good I'll add some. FWIW I tested this with one of my own projects and it works as expected.

…c hooks listed in setup.cfg first, then calls the old run_entry_points to run any globally-installed entry points.
logger.warning('cannot find %s hook: %s; skipping...',
hook_name, e.args[0])
for hook in hooks:
hook(data)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Might be nice to add some additional error handling for when hooks fail; ditto for entry_points.

@reinout
Copy link
Collaborator

reinout commented Jul 11, 2012

I like the idea and I can see its use. Simple fix that works well.

One thing I dislike is the 'package_dir' name: it is a bit too generic. The term as such is right, but it doesn't tell you that it is really for the hooks. 'extra_package_dir_for_hooks' is a bit long, though. Do you have an alternative here?

So... the approach is right, so could you add that test you mentioned? And perhaps a two-line note in the readme?

@embray
Copy link
Contributor Author

embray commented Jul 11, 2012

Cool, I'm glad you like it. As for package_dir I agree it's a bit vague in that context. Maybe just hook_package_dir? That way it suggests one could have releaser hooks even in a separate directory that isn't installed with the rest of the source code.

I'll see about coming up with some tests, and adding docs.

@embray
Copy link
Contributor Author

embray commented Jul 11, 2012

By the way, I don't know if this is the best place to mention this, but I'll probably be submitting a number of patches over the coming weeks. I've been using zest.releaser for like a year now and rather like it. And while I agree with most of the default assumptions, there are some annoyances. Most of my patches will be geared toward making it a little easier to customize its behavior (while keeping the existing defaults, since I understand they're chosen for what works for Zest software).

But if some of my patches aren't accepted for whatever reason is there any problem with me releasing a fork of some sort under a different name?

@reinout
Copy link
Collaborator

reinout commented Jul 12, 2012

hook_package_dir is fine.

Regarding forking: it is all GPL, so you can fork to your heart's delight and release it under a different name.

It ought to work just fine for most projects, though. I set it up with an eye on the Plone codebase and extensions and whatever of that time, so it is not just zest software's default. There are a couple of things I know of that aren't supported, but mostly because nobody bothered to implement it.

So... could you add the points you have as tickets? Perhaps they're easy to support anyway.

@embray
Copy link
Contributor Author

embray commented Jul 12, 2012

I'm not really a Zope/Plone guy so I'm not entirely familiar with this test framework. I understand how it works, but I'm wondering: What's the best way to run a single test from the suite, rather than having to run the entire suite just to test the test?

As for forking, I know it's GPL but I still think it's polite to ask :) I'll see about adding tickets for the other issues I want to bring up. Thanks again!

@mauritsvanrees
Copy link
Member

To run only the tests in the git.txt file, run bin/test -t git.txt

bin/test --help has more options, but this is probably the most important one in this case.

@embray
Copy link
Contributor Author

embray commented Aug 22, 2012

Thanks--I'm not used to finding tests in text files like that, though I guess that should have been fairly clear. And no, I haven't abandoned this, just got side-tracked by work and stuff :)

…r. Added a test (based on the functional-git.txt test with some redundancies removed) that tests whether the hooks are run and in the right places.
@travisbot
Copy link

This pull request fails (merged a9168da into 060b3f4).

@mauritsvanrees
Copy link
Member

I have just merged this. Sorry it took so long.

One thing I am wondering though: can we not use the existing package_dir option from setup.py instead of hook_package_dir in setup.cfg?

@embray
Copy link
Contributor Author

embray commented Nov 5, 2012

No problem--thanks for merging. I have other stuff I wanted to submit for zest.releaser but haven't had time to work on it :/ Hopefully soon. The setup.py package_dir option could be used. The reason I added a separate option is that one might hypothetically want to put releaser hooks in a separate directory outside their main package, since there's not necessarily any reason to install them with the package. But the package_dir might not be bad to include as a default?

@mauritsvanrees
Copy link
Member

Seems a good default yes.

@mauritsvanrees
Copy link
Member

I have finally released 3.42 with this pull request.

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.

None yet

4 participants