Setup pre-commit with useful plugins

[jcpunk]

This adds support for pre-commit to the repo which can greatly simplify consistent code practices.

[DrDaveD]

The new pre-commit check fails.

What was the inspiration for this?

[brianhlin]

I've always wanted to play around with pre-commit and black but have always been hesitant because I haven't gone through the trouble of making sure black doesn't royally screw up git blame. If we address that and make sure that we still support Python 3.6, I could be open to this.

[brianhlin]

Oh and I missed that you're using the pre-commit action,

this action is in maintenance-only mode and will not be accepting new features.

generally you want to use pre-commit.ci which is faster and has more features.

[bbockelm]

@brianhlin - you want https://docs.github.com/en/repositories/working-with-files/using-files/viewing-a-file#ignore-commits-in-the-blame-view .

And yes, precommit tries to bump you to their CI. I tend to just ignore that.

[jcpunk]

The new pre-commit check fails.

What was the inspiration for this?

If you're interested in going with these tools, some automation to keep folks in sync is usually helpful.

[DrDaveD]

We can't use it if it fails the check. Do you plan to fix it?

The tools in this package are quite small and hardly seem worth spending much effort on.

[jcpunk]

Fixing the check is trivial; I can get that done if this is a direction folks are interested in.

[DrDaveD]

I think that as long as you've done the work we might as well include it. It sounds like the Brians are in favor too.

[jcpunk]

float_to_top = "True"

Might need to be removed because of osg-cert-request and osg-incommon-request depending on how you're calling them.

I'd be tempted to use entrypoint: [console_scripts](https://python-packaging.readthedocs.io/en/latest/command-line-scripts.html) to generate them more dynamically.

[brianhlin]

@jcpunk can you add a .git-blame-ignore-revs file including bced63736532f958c7dd1c12a36b042074bfad70?

[DrDaveD]

float_to_top = "True"

Might need to be removed because of osg-cert-request and osg-incommon-request depending on how you're calling them.

What do you mean? What type of way of calling them would be tripped up by float_to_top = "True".

I'd be tempted to use entrypoint: console_scripts to generate them more dynamically.

How does that help?

[jcpunk]

What do you mean? What type of way of calling them would be tripped up by float_to_top = "True".

It looks like the scripts are expecting to adjust sys.path before finishing the imports. With float_to_top = True the isort tool will move all the imports to the top of the scope. This may not be what you're expecting for these scripts.

The setuptools console_scripts will generate these "/usr/bin/" scripts dynamically so you don't need to write/maintain/separate these out. Use of these setuptools hooks should in theory reduce your long term burden. We've had good success with those in the HepCloud DecisionEngine.

[jcpunk]

Added the ignore revs file

[brianhlin]

@jcpunk it looks like those Python path hacks are just that, hacks to make unit testing happy and to make life easier when running in a virtualenv. I would definitely like both to continue working so if you've got alternate suggestions for those use cases, I'm all ears!

[jcpunk]

I think my current PR should leave them in place.

…

On Fri, Mar 29, 2024, 4:26 PM Brian Lin ***@***.***> wrote: @jcpunk <https://github.com/jcpunk> it looks like those Python path hacks are just that, hacks to make unit testing happy and to make life easier when running in a virtualenv. I would definitely like both to continue working so if you've got alternate suggestions for those use cases, I'm all ears! — Reply to this email directly, view it on GitHub <#85 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AA2673SAWYIDC7D73U2QANDY2XMCBAVCNFSM6AAAAABFD5X646VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAMRXG42TCOJWGA> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[brianhlin]

In that case, I'm happy with this unless @DrDaveD has any other thoughts

[DrDaveD]

Looks OK to me