pre-commit hooks?

41 views
Skip to first unread message

Ilia Kurenkov

unread,
Oct 16, 2019, 3:20:59 PM10/16/19
to nltk-dev
Now that we are using black as the code formatter, would it make sense to have it run automatically when folks submit their code?

I fear that without some kind of style enforcement the style will deteriorate over time and we'll have to run the tool on the whole repo again and that really messes with git history and keeping track of who actually introduced a change.

The downside of something like pre-commit is that folks will have to install it and understand how it works, at least on a basic level.
We could try automating the setup with a makefile or with scripts of course, or we could look for alternatives.

For instance, we could have some kind of checks set up in the CI pipeline that block a PR if it doesn't satisfy our style requirements.

Steven Bird

unread,
Oct 22, 2019, 3:51:22 AM10/22/19
to nltk-dev
Interesting idea... I expect Travis supports style testing, so we might just have to reconfigure that, instead of setting up something new?

-Steven



--
You received this message because you are subscribed to the Google Groups "nltk-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to nltk-dev+u...@googlegroups.com.
To view this discussion on the web, visit https://groups.google.com/d/msgid/nltk-dev/8f4ac57c-0ebf-4c5c-892c-6673144104a2%40googlegroups.com.

Ilia Kurenkov

unread,
Oct 23, 2019, 5:00:10 PM10/23/19
to nltk-dev
Sure! I'm all in favor of something that for the least amount of effort achieves the goal, which is maintaining the style throughout. The hooks were definitely just one idea to get the ball rolling on the more general topic.

Maybe I should have named the thread better :)

On Tuesday, October 22, 2019 at 9:51:22 AM UTC+2, Steven Bird wrote:
Interesting idea... I expect Travis supports style testing, so we might just have to reconfigure that, instead of setting up something new?

-Steven



On Thu, 17 Oct 2019 at 04:51, Ilia Kurenkov <ilia.k...@gmail.com> wrote:
Now that we are using black as the code formatter, would it make sense to have it run automatically when folks submit their code?

I fear that without some kind of style enforcement the style will deteriorate over time and we'll have to run the tool on the whole repo again and that really messes with git history and keeping track of who actually introduced a change.

The downside of something like pre-commit is that folks will have to install it and understand how it works, at least on a basic level.
We could try automating the setup with a makefile or with scripts of course, or we could look for alternatives.

For instance, we could have some kind of checks set up in the CI pipeline that block a PR if it doesn't satisfy our style requirements.

--
You received this message because you are subscribed to the Google Groups "nltk-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to nltk...@googlegroups.com.

Steven Bird

unread,
Oct 23, 2019, 6:24:12 PM10/23/19
to nltk-dev
I'm happy to go ahead with this, if someone would like to take it on?

To unsubscribe from this group and stop receiving emails from it, send an email to nltk-dev+u...@googlegroups.com.
To view this discussion on the web, visit https://groups.google.com/d/msgid/nltk-dev/5a20cd54-5414-4c3e-a9bb-5b852d1749e8%40googlegroups.com.

Ilia Kurenkov

unread,
Oct 24, 2019, 11:57:21 AM10/24/19
to nltk-dev
Would this someone need to have admin access to Travis?
I'm asking for general information, I don't think I'll have time in the near future to learn enough about the workings of travis to mess with this.

Steven Bird

unread,
Oct 24, 2019, 4:39:47 PM10/24/19
to nltk-dev
Yes, we can do that.

To unsubscribe from this group and stop receiving emails from it, send an email to nltk-dev+u...@googlegroups.com.
To view this discussion on the web, visit https://groups.google.com/d/msgid/nltk-dev/107556bc-a001-496c-9f65-47910db7ffd7%40googlegroups.com.
Reply all
Reply to author
Forward
0 new messages