Conversation
|
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
|
Whoops - the failing "Test utility scripts" is my fault. I set up that workflow to run when the "Infrastructure" tag is present. But it works by diffing against main, and in this case I modified a bunch of other notebooks. I have to rethink when to run those tests... |
for more information, see https://pre-commit.ci
With the goal of making sure we don't increase the repo size by merging commits with outputs to I do wonder how GitHub records authorship for these squashes, since both the original author and pre-commit.ci's bot are "authors", and of course the person who triggers the merge is a committer. I think it'll work out, but it's something to keep an eye on. |
jonathansick
left a comment
There was a problem hiding this comment.
Excellent! I like this a lot, thanks for doing it.
One thing I like to do for pre-commit repos is add a make init command to the Makefile. It'll look a bit like this:
.PHONY: init
init:
pip install --U pre-commit
pre-commit install
In the make init you can also install dependencies, etc. I just find it easier to document running a make command rather than explaining pre-commit sometimes.
|
I haven't reviewed in detail but I like the approach. |
I don't know if even squashing will remove big commits entirely....it might, but haven't experimented. I've used this for cleaning out big files/commits in the past: https://rtyley.github.io/bfg-repo-cleaner/ |
Aha, I like it! I guess we could also add a
I think if the commit that added the big-ness was in the same set of commits as the squash, it will remove the big-ness? But I could be wrong! |
Yeah, if the squashed commit doesn't contain that big content, that big content will never make it to the |
This adds a config file for pre-commit, which includes a run of
nbstripoutto ensure that cell output has been removed along with python version, TOC metadata, and kernelspec info. I think this solves astropy-learn/nbcollection#16!I also configured pre-commit.ci to run on PRs in this repo. We need to add some info about the expected contributor workflow. Since pre-commit.ci will push changes to contributor branches, this means contributors need to make sure to pull before they modify notebooks contributed in PRs. It also means that we should squash-and-merge by default to make sure cell outputs don't end up in the commit history. I think this is still simpler than asking contributors to rebase if they accidentally commit cell output. I added TODO items here to remind me to document this.
One question: should we disable merge commit merging, and only allow squash or rebase merges?
Fixes #518